* [PATCH 4.18 20/22] Bluetooth: hidp: buffer overflow in hidp_process_report
From: Greg Kroah-Hartman @ 2018-08-16 18:45 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Mark Salyzyn, Marcel Holtmann,
Johan Hedberg, David S. Miller, Kees Cook, Benjamin Tissoires,
linux-bluetooth, netdev, security, kernel-team
In-Reply-To: <20180816171556.502583508@linuxfoundation.org>
4.18-stable review patch. If anyone has any objections, please let me know.
------------------
From: Mark Salyzyn <salyzyn@android.com>
commit 7992c18810e568b95c869b227137a2215702a805 upstream.
CVE-2018-9363
The buffer length is unsigned at all layers, but gets cast to int and
checked in hidp_process_report and can lead to a buffer overflow.
Switch len parameter to unsigned int to resolve issue.
This affects 3.18 and newer kernels.
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Fixes: a4b1b5877b514b276f0f31efe02388a9c2836728 ("HID: Bluetooth: hidp: make sure input buffers are big enough")
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-bluetooth@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: security@kernel.org
Cc: kernel-team@android.com
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/bluetooth/hidp/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -431,8 +431,8 @@ static void hidp_del_timer(struct hidp_s
del_timer(&session->timer);
}
-static void hidp_process_report(struct hidp_session *session,
- int type, const u8 *data, int len, int intr)
+static void hidp_process_report(struct hidp_session *session, int type,
+ const u8 *data, unsigned int len, int intr)
{
if (len > HID_MAX_BUFFER_SIZE)
len = HID_MAX_BUFFER_SIZE;
^ permalink raw reply
* [PATCH 4.17 19/21] Bluetooth: hidp: buffer overflow in hidp_process_report
From: Greg Kroah-Hartman @ 2018-08-16 18:45 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Mark Salyzyn, Marcel Holtmann,
Johan Hedberg, David S. Miller, Kees Cook, Benjamin Tissoires,
linux-bluetooth, netdev, security, kernel-team
In-Reply-To: <20180816171612.136242278@linuxfoundation.org>
4.17-stable review patch. If anyone has any objections, please let me know.
------------------
From: Mark Salyzyn <salyzyn@android.com>
commit 7992c18810e568b95c869b227137a2215702a805 upstream.
CVE-2018-9363
The buffer length is unsigned at all layers, but gets cast to int and
checked in hidp_process_report and can lead to a buffer overflow.
Switch len parameter to unsigned int to resolve issue.
This affects 3.18 and newer kernels.
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Fixes: a4b1b5877b514b276f0f31efe02388a9c2836728 ("HID: Bluetooth: hidp: make sure input buffers are big enough")
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-bluetooth@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: security@kernel.org
Cc: kernel-team@android.com
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/bluetooth/hidp/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -431,8 +431,8 @@ static void hidp_del_timer(struct hidp_s
del_timer(&session->timer);
}
-static void hidp_process_report(struct hidp_session *session,
- int type, const u8 *data, int len, int intr)
+static void hidp_process_report(struct hidp_session *session, int type,
+ const u8 *data, unsigned int len, int intr)
{
if (len > HID_MAX_BUFFER_SIZE)
len = HID_MAX_BUFFER_SIZE;
^ permalink raw reply
* [PATCH 4.14 20/22] Bluetooth: hidp: buffer overflow in hidp_process_report
From: Greg Kroah-Hartman @ 2018-08-16 18:45 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Mark Salyzyn, Marcel Holtmann,
Johan Hedberg, David S. Miller, Kees Cook, Benjamin Tissoires,
linux-bluetooth, netdev, security, kernel-team
In-Reply-To: <20180816171622.831964729@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Mark Salyzyn <salyzyn@android.com>
commit 7992c18810e568b95c869b227137a2215702a805 upstream.
CVE-2018-9363
The buffer length is unsigned at all layers, but gets cast to int and
checked in hidp_process_report and can lead to a buffer overflow.
Switch len parameter to unsigned int to resolve issue.
This affects 3.18 and newer kernels.
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Fixes: a4b1b5877b514b276f0f31efe02388a9c2836728 ("HID: Bluetooth: hidp: make sure input buffers are big enough")
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-bluetooth@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: security@kernel.org
Cc: kernel-team@android.com
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/bluetooth/hidp/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -431,8 +431,8 @@ static void hidp_del_timer(struct hidp_s
del_timer(&session->timer);
}
-static void hidp_process_report(struct hidp_session *session,
- int type, const u8 *data, int len, int intr)
+static void hidp_process_report(struct hidp_session *session, int type,
+ const u8 *data, unsigned int len, int intr)
{
if (len > HID_MAX_BUFFER_SIZE)
len = HID_MAX_BUFFER_SIZE;
^ permalink raw reply
* [PATCH 4.9 13/15] Bluetooth: hidp: buffer overflow in hidp_process_report
From: Greg Kroah-Hartman @ 2018-08-16 18:42 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Mark Salyzyn, Marcel Holtmann,
Johan Hedberg, David S. Miller, Kees Cook, Benjamin Tissoires,
linux-bluetooth, netdev, security, kernel-team
In-Reply-To: <20180816171625.340082081@linuxfoundation.org>
4.9-stable review patch. If anyone has any objections, please let me know.
------------------
From: Mark Salyzyn <salyzyn@android.com>
commit 7992c18810e568b95c869b227137a2215702a805 upstream.
CVE-2018-9363
The buffer length is unsigned at all layers, but gets cast to int and
checked in hidp_process_report and can lead to a buffer overflow.
Switch len parameter to unsigned int to resolve issue.
This affects 3.18 and newer kernels.
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Fixes: a4b1b5877b514b276f0f31efe02388a9c2836728 ("HID: Bluetooth: hidp: make sure input buffers are big enough")
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-bluetooth@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: security@kernel.org
Cc: kernel-team@android.com
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/bluetooth/hidp/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -431,8 +431,8 @@ static void hidp_del_timer(struct hidp_s
del_timer(&session->timer);
}
-static void hidp_process_report(struct hidp_session *session,
- int type, const u8 *data, int len, int intr)
+static void hidp_process_report(struct hidp_session *session, int type,
+ const u8 *data, unsigned int len, int intr)
{
if (len > HID_MAX_BUFFER_SIZE)
len = HID_MAX_BUFFER_SIZE;
^ permalink raw reply
* [PATCH 4.4 11/13] Bluetooth: hidp: buffer overflow in hidp_process_report
From: Greg Kroah-Hartman @ 2018-08-16 18:41 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Mark Salyzyn, Marcel Holtmann,
Johan Hedberg, David S. Miller, Kees Cook, Benjamin Tissoires,
linux-bluetooth, netdev, security, kernel-team
In-Reply-To: <20180816171628.554317013@linuxfoundation.org>
4.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Mark Salyzyn <salyzyn@android.com>
commit 7992c18810e568b95c869b227137a2215702a805 upstream.
CVE-2018-9363
The buffer length is unsigned at all layers, but gets cast to int and
checked in hidp_process_report and can lead to a buffer overflow.
Switch len parameter to unsigned int to resolve issue.
This affects 3.18 and newer kernels.
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Fixes: a4b1b5877b514b276f0f31efe02388a9c2836728 ("HID: Bluetooth: hidp: make sure input buffers are big enough")
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-bluetooth@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: security@kernel.org
Cc: kernel-team@android.com
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/bluetooth/hidp/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -431,8 +431,8 @@ static void hidp_del_timer(struct hidp_s
del_timer(&session->timer);
}
-static void hidp_process_report(struct hidp_session *session,
- int type, const u8 *data, int len, int intr)
+static void hidp_process_report(struct hidp_session *session, int type,
+ const u8 *data, unsigned int len, int intr)
{
if (len > HID_MAX_BUFFER_SIZE)
len = HID_MAX_BUFFER_SIZE;
^ permalink raw reply
* [PATCH 3.18 15/15] Bluetooth: hidp: buffer overflow in hidp_process_report
From: Greg Kroah-Hartman @ 2018-08-16 18:41 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Mark Salyzyn, Marcel Holtmann,
Johan Hedberg, David S. Miller, Kees Cook, Benjamin Tissoires,
linux-bluetooth, netdev, security, kernel-team
In-Reply-To: <20180816171633.546734046@linuxfoundation.org>
3.18-stable review patch. If anyone has any objections, please let me know.
------------------
From: Mark Salyzyn <salyzyn@android.com>
commit 7992c18810e568b95c869b227137a2215702a805 upstream.
CVE-2018-9363
The buffer length is unsigned at all layers, but gets cast to int and
checked in hidp_process_report and can lead to a buffer overflow.
Switch len parameter to unsigned int to resolve issue.
This affects 3.18 and newer kernels.
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Fixes: a4b1b5877b514b276f0f31efe02388a9c2836728 ("HID: Bluetooth: hidp: make sure input buffers are big enough")
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-bluetooth@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: security@kernel.org
Cc: kernel-team@android.com
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/bluetooth/hidp/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -416,8 +416,8 @@ static void hidp_del_timer(struct hidp_s
del_timer(&session->timer);
}
-static void hidp_process_report(struct hidp_session *session,
- int type, const u8 *data, int len, int intr)
+static void hidp_process_report(struct hidp_session *session, int type,
+ const u8 *data, unsigned int len, int intr)
{
if (len > HID_MAX_BUFFER_SIZE)
len = HID_MAX_BUFFER_SIZE;
^ permalink raw reply
* Re: [PATCH stable 4.4 0/9] fix SegmentSmack in stable branch (CVE-2018-5390)
From: Greg KH @ 2018-08-16 15:24 UTC (permalink / raw)
To: Michal Kubecek
Cc: maowenan, dwmw2, netdev, eric.dumazet, edumazet, davem, ycheng,
jdw, stable, Takashi Iwai
In-Reply-To: <20180816123356.47iq3bts427cpptx@unicorn.suse.cz>
On Thu, Aug 16, 2018 at 02:33:56PM +0200, Michal Kubecek wrote:
> On Thu, Aug 16, 2018 at 08:05:50PM +0800, maowenan wrote:
> > On 2018/8/16 19:39, Michal Kubecek wrote:
> > >
> > > I suspect you may be doing something wrong with your tests. I checked
> > > the segmentsmack testcase and the CPU utilization on receiving side
> > > (with sending 10 times as many packets as default) went down from ~100%
> > > to ~3% even when comparing what is in stable 4.4 now against older 4.4
> > > kernel.
> >
> > There seems no obvious problem when you send packets with default
> > parameter in Segmentsmack POC, Which is also very related with your
> > server's hardware configuration. Please try with below parameter to
> > form OFO packets
>
> I did and even with these (questionable, see below) changes, I did not
> get more than 10% (of one core) by receiving ksoftirqd.
>
> > for (i = 0; i < 1024; i++) // 128->1024
> ...
> > usleep(10*1000); // Adjust this and packet count to match the target!, sleep 100ms->10ms
>
> The comment in the testcase source suggests to do _one_ of these two
> changes so that you generate 10 times as many packets as the original
> testcase. You did both so that you end up sending 102400 packets per
> second. With 55 byte long packets, this kind of attack requires at least
> 5.5 MB/s (44 Mb/s) of throughput. This is no longer a "low packet rate
> DoS", I'm afraid.
>
> Anyway, even at this rate, I only get ~10% of one core (Intel E5-2697).
>
> What I can see, though, is that with current stable 4.4 code, modified
> testcase which sends something like
>
> 2:3, 3:4, ..., 3001:3002, 3003:3004, 3004:3005, ... 6001:6002, ...
>
> I quickly eat 6 MB of memory for receive queue of one socket while
> earlier 4.4 kernels only take 200-300 KB. I didn't test latest 4.4 with
> Takashi's follow-up yet but I'm pretty sure it will help while
> preserving nice performance when using the original segmentsmack
> testcase (with increased packet ratio).
Ok, for now I've applied Takashi's fix to the 4.4 stable queue and will
push out a new 4.4-rc later tonight. Can everyone standardize on that
and test and let me know if it does, or does not, fix the reported
issues?
If not, we can go from there and evaluate this much larger patch series.
But let's try the simple thing first.
thanks,
greg k-h
^ permalink raw reply
* Re: [iproute PATCH v4] Make colored output configurable
From: Phil Sutter @ 2018-08-16 15:00 UTC (permalink / raw)
To: David Ahern; +Cc: Stephen Hemminger, netdev, Till Maas
In-Reply-To: <46aa7a15-0317-10e4-0404-580b38fe2efb@gmail.com>
On Thu, Aug 16, 2018 at 07:06:07AM -0600, David Ahern wrote:
> On 8/16/18 3:37 AM, Phil Sutter wrote:
> > Allow for -color={never,auto,always} to have colored output disabled,
> > enabled only if stdout is a terminal or enabled regardless of stdout
> > state.
> >
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> > Changes since v1:
> > - Allow to override isatty() check by specifying '-color' flag more than
> > once.
> > - Document new behaviour in man pages.
> >
> > Changes since v2:
> > - Implement new -color=foo syntax.
> > - Update commit message and man page texts accordingly.
> >
> > Changes since v3:
> > - Fix typo in tc/tc.c causing compile error.
> > ---
> > bridge/bridge.c | 3 +--
> > include/color.h | 7 +++++++
> > ip/ip.c | 3 +--
> > lib/color.c | 33 ++++++++++++++++++++++++++++++++-
> > man/man8/bridge.8 | 13 +++++++++++--
> > man/man8/ip.8 | 13 +++++++++++--
> > man/man8/tc.8 | 13 +++++++++++--
> > tc/tc.c | 3 +--
> > 8 files changed, 75 insertions(+), 13 deletions(-)
> >
> > diff --git a/bridge/bridge.c b/bridge/bridge.c
> > index 451d684e0bcfd..e35e5bdf7fb30 100644
> > --- a/bridge/bridge.c
> > +++ b/bridge/bridge.c
> > @@ -173,8 +173,7 @@ main(int argc, char **argv)
> > NEXT_ARG();
> > if (netns_switch(argv[1]))
> > exit(-1);
> > - } else if (matches(opt, "-color") == 0) {
> > - ++color;
> > + } else if (matches_color(opt, &color) == 0) {
> > } else if (matches(opt, "-compressvlans") == 0) {
> > ++compress_vlans;
> > } else if (matches(opt, "-force") == 0) {
> > diff --git a/include/color.h b/include/color.h
> > index 4f2c918db7e43..42038dc2e7f87 100644
> > --- a/include/color.h
> > +++ b/include/color.h
> > @@ -12,8 +12,15 @@ enum color_attr {
> > COLOR_NONE
> > };
> >
> > +enum color_opt {
> > + COLOR_OPT_NEVER = 0,
> > + COLOR_OPT_AUTO = 1,
> > + COLOR_OPT_ALWAYS = 2
> > +};
>
> The order of AUTO and ALWAYS is backwards. Existing users who do
> something like:
Ordering color_opt this way felt natural since the default (0)
automatically matches the existing default (no colors).
> ip -c addr list | less -R
> or
> ip -c addr list > /tmp/addr
> less -R /tmp/addr
>
> should not be affected by this change. That is an existing command that
> works and should continue to work the same after this change.
>
> Users who add -c but don't want the codes if stdout is not a tty are the
> ones who should be doing something new - be it adding another -c or
> using -c=auto.
My code is correct in that regard: Plain -c[olor] is equivalent to
-color=always.
Cheers, Phil
^ permalink raw reply
* Re: [PATCH] netfilter/x_tables: do not fail xt_alloc_table_info too easilly
From: Pablo Neira Ayuso @ 2018-08-16 16:41 UTC (permalink / raw)
To: Michal Hocko
Cc: Florian Westphal, Vlastimil Babka, Georgi Nikolov, Andrew Morton,
David S. Miller, netfilter-devel, coreteam, netdev, linux-kernel,
linux-mm, Michal Hocko
In-Reply-To: <20180807195400.23687-1-mhocko@kernel.org>
On Tue, Aug 07, 2018 at 09:54:00PM +0200, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> eacd86ca3b03 ("net/netfilter/x_tables.c: use kvmalloc()
> in xt_alloc_table_info()") has unintentionally fortified
> xt_alloc_table_info allocation when __GFP_RETRY has been dropped from
> the vmalloc fallback. Later on there was a syzbot report that this
> can lead to OOM killer invocations when tables are too large and
> 0537250fdc6c ("netfilter: x_tables: make allocation less aggressive")
> has been merged to restore the original behavior. Georgi Nikolov however
> noticed that he is not able to install his iptables anymore so this can
> be seen as a regression.
>
> The primary argument for 0537250fdc6c was that this allocation path
> shouldn't really trigger the OOM killer and kill innocent tasks. On the
> other hand the interface requires root and as such should allow what the
> admin asks for. Root inside a namespaces makes this more complicated
> because those might be not trusted in general. If they are not then such
> namespaces should be restricted anyway. Therefore drop the __GFP_NORETRY
> and replace it by __GFP_ACCOUNT to enfore memcg constrains on it.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH stable 4.4 0/9] fix SegmentSmack in stable branch (CVE-2018-5390)
From: Greg KH @ 2018-08-16 16:20 UTC (permalink / raw)
To: Michal Kubecek
Cc: maowenan, dwmw2, netdev, eric.dumazet, edumazet, davem, ycheng,
jdw, stable, Takashi Iwai
In-Reply-To: <20180816160616.u3refk4mqpyqagzi@unicorn.suse.cz>
On Thu, Aug 16, 2018 at 06:06:16PM +0200, Michal Kubecek wrote:
> > If not, we can go from there and evaluate this much larger patch
> > series. But let's try the simple thing first.
>
> At high packet rates (say 30K pkt/s and more), we can still saturate the
> CPU. This is also mentioned in the announcement with claim that switch
> to rbtree based queue would be necessary to fully address that. My tests
> seem to confirm that but I'm still not sure it is worth backporting
> something as intrusive into stable 4.4.
No, it is not. If you worry about those things, you should not be
running a 4.4 kernel, use 4.14 or newer please.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH stable 4.4 0/9] fix SegmentSmack in stable branch (CVE-2018-5390)
From: Michal Kubecek @ 2018-08-16 16:06 UTC (permalink / raw)
To: Greg KH
Cc: maowenan, dwmw2, netdev, eric.dumazet, edumazet, davem, ycheng,
jdw, stable, Takashi Iwai
In-Reply-To: <20180816152409.GK10648@kroah.com>
On Thu, Aug 16, 2018 at 05:24:09PM +0200, Greg KH wrote:
> On Thu, Aug 16, 2018 at 02:33:56PM +0200, Michal Kubecek wrote:
> >
> > Anyway, even at this rate, I only get ~10% of one core (Intel E5-2697).
> >
> > What I can see, though, is that with current stable 4.4 code, modified
> > testcase which sends something like
> >
> > 2:3, 3:4, ..., 3001:3002, 3003:3004, 3004:3005, ... 6001:6002, ...
> >
> > I quickly eat 6 MB of memory for receive queue of one socket while
> > earlier 4.4 kernels only take 200-300 KB. I didn't test latest 4.4 with
> > Takashi's follow-up yet but I'm pretty sure it will help while
> > preserving nice performance when using the original segmentsmack
> > testcase (with increased packet ratio).
>
> Ok, for now I've applied Takashi's fix to the 4.4 stable queue and will
> push out a new 4.4-rc later tonight. Can everyone standardize on that
> and test and let me know if it does, or does not, fix the reported
> issues?
I did repeat the tests with Takashi's fix and the CPU utilization is
similar to what we have now, i.e. 3-5% with 10K pkt/s. I could still
saturate one CPU somewhere around 50K pkt/s but that already requires
2.75 MB/s (22 Mb/s) of throughput. (My previous tests with Mao Wenan's
changes in fact used lower speeds as the change from 128 to 1024 would
need to be done in two places.)
Where Takashi's patch does help is that it does not prevent collapsing
of ranges of adjacent segments with total length shorter than ~4KB. It
took more time to verify: it cannot be checked by watching the socket
memory consumption with ss as tcp_collapse_ofo_queue isn't called until
we reach the limits. So I needed to trace when and how tcp_collpse() is
called with both current stable 4.4 code and one with Takashi's fix.
> If not, we can go from there and evaluate this much larger patch
> series. But let's try the simple thing first.
At high packet rates (say 30K pkt/s and more), we can still saturate the
CPU. This is also mentioned in the announcement with claim that switch
to rbtree based queue would be necessary to fully address that. My tests
seem to confirm that but I'm still not sure it is worth backporting
something as intrusive into stable 4.4.
Michal Kubecek
^ permalink raw reply
* Re: [iproute PATCH v4] Make colored output configurable
From: David Ahern @ 2018-08-16 13:06 UTC (permalink / raw)
To: Phil Sutter, Stephen Hemminger; +Cc: netdev, Till Maas
In-Reply-To: <20180816093703.23022-1-phil@nwl.cc>
On 8/16/18 3:37 AM, Phil Sutter wrote:
> Allow for -color={never,auto,always} to have colored output disabled,
> enabled only if stdout is a terminal or enabled regardless of stdout
> state.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since v1:
> - Allow to override isatty() check by specifying '-color' flag more than
> once.
> - Document new behaviour in man pages.
>
> Changes since v2:
> - Implement new -color=foo syntax.
> - Update commit message and man page texts accordingly.
>
> Changes since v3:
> - Fix typo in tc/tc.c causing compile error.
> ---
> bridge/bridge.c | 3 +--
> include/color.h | 7 +++++++
> ip/ip.c | 3 +--
> lib/color.c | 33 ++++++++++++++++++++++++++++++++-
> man/man8/bridge.8 | 13 +++++++++++--
> man/man8/ip.8 | 13 +++++++++++--
> man/man8/tc.8 | 13 +++++++++++--
> tc/tc.c | 3 +--
> 8 files changed, 75 insertions(+), 13 deletions(-)
>
> diff --git a/bridge/bridge.c b/bridge/bridge.c
> index 451d684e0bcfd..e35e5bdf7fb30 100644
> --- a/bridge/bridge.c
> +++ b/bridge/bridge.c
> @@ -173,8 +173,7 @@ main(int argc, char **argv)
> NEXT_ARG();
> if (netns_switch(argv[1]))
> exit(-1);
> - } else if (matches(opt, "-color") == 0) {
> - ++color;
> + } else if (matches_color(opt, &color) == 0) {
> } else if (matches(opt, "-compressvlans") == 0) {
> ++compress_vlans;
> } else if (matches(opt, "-force") == 0) {
> diff --git a/include/color.h b/include/color.h
> index 4f2c918db7e43..42038dc2e7f87 100644
> --- a/include/color.h
> +++ b/include/color.h
> @@ -12,8 +12,15 @@ enum color_attr {
> COLOR_NONE
> };
>
> +enum color_opt {
> + COLOR_OPT_NEVER = 0,
> + COLOR_OPT_AUTO = 1,
> + COLOR_OPT_ALWAYS = 2
> +};
The order of AUTO and ALWAYS is backwards. Existing users who do
something like:
ip -c addr list | less -R
or
ip -c addr list > /tmp/addr
less -R /tmp/addr
should not be affected by this change. That is an existing command that
works and should continue to work the same after this change.
Users who add -c but don't want the codes if stdout is not a tty are the
ones who should be doing something new - be it adding another -c or
using -c=auto.
^ permalink raw reply
* Re: [PATCH] net: phy: add tracepoints
From: Steven Rostedt @ 2018-08-16 12:46 UTC (permalink / raw)
To: Tobias Waldekranz
Cc: Tobias Waldekranz, Andrew Lunn, Florian Fainelli, David S. Miller,
Ingo Molnar, open list, open list:ETHERNET PHY LIBRARY
In-Reply-To: <20180816093056.2485-1-tobias@waldekranz.com>
On Thu, 16 Aug 2018 11:30:53 +0200
Tobias Waldekranz <tobias@waldekranz.com> wrote:
> Two tracepoints for now:
>
> * `phy_interrupt` Pretty self-explanatory.
>
> * `phy_state_change` Whenever the PHY's state machine is run, trace
> the old and the new state.
>From a tracing perspective, this patch looks fine,
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
But below I have a possible improvement.
>
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
> drivers/net/phy/phy.c | 4 +++
> include/trace/events/phy.h | 68 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 72 insertions(+)
> create mode 100644 include/trace/events/phy.h
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 9aabfa1a455a..8d22926f3962 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -38,6 +38,9 @@
>
> #include <asm/irq.h>
>
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/phy.h>
> +
> #define PHY_STATE_STR(_state) \
> case PHY_##_state: \
> return __stringify(_state); \
> @@ -1039,6 +1042,7 @@ void phy_state_machine(struct work_struct *work)
> if (err < 0)
> phy_error(phydev);
>
> + trace_phy_state_change(phydev, old_state);
I see that you are passing in a enum phy_state.
> if (old_state != phydev->state)
> phydev_dbg(phydev, "PHY state change %s -> %s\n",
> phy_state_to_str(old_state),
> diff --git a/include/trace/events/phy.h b/include/trace/events/phy.h
> new file mode 100644
> index 000000000000..7ba6c0dda47e
> --- /dev/null
> +++ b/include/trace/events/phy.h
> @@ -0,0 +1,68 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM phy
You can add this:
#define PHY_STATE_ENUMS \
EM( DOWN ) \
EM( STARTING ) \
EM( READY ) \
EM( PENDING ) \
EM( UP ) \
EM( AN ) \
EM( RUNNING ) \
EM( NOLINK ) \
EM( FORCING ) \
EM( CHANGELINK )\
EM( HALTED ) \
EMe(RESUMING)
#undef EM
#undef EMe
#define EM(a) TRACE_DEFINE_ENUM( PHY_##a );
#define EMe(a) TRACE_DEFINE_ENUM( PHY_##a );
PHY_STATE_ENUMS
#undef EM
#undef EMe
#define EM(a) { PHY_##a, #a },
#define EMe(a) { PHY_##a, #a }
> +
> +#if !defined(_TRACE_PHY_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_PHY_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(phy_interrupt,
> + TP_PROTO(int irq, struct phy_device *phydev),
> + TP_ARGS(irq, phydev),
> + TP_STRUCT__entry(
> + __field(int, irq)
> + __field(int, addr)
> + __field(int, state)
> + __array(char, ifname, IFNAMSIZ)
> + ),
> + TP_fast_assign(
> + __entry->irq = irq;
> + __entry->addr = phydev->mdio.addr;
> + __entry->state = phydev->state;
> + if (phydev->attached_dev)
> + memcpy(__entry->ifname,
> + netdev_name(phydev->attached_dev),
> + IFNAMSIZ);
> + else
> + memset(__entry->ifname, 0, IFNAMSIZ);
> + ),
> + TP_printk("phy-%d-irq irq=%d ifname=%16s state=%d",
Here you can have "state=%s"
> + __entry->addr,
> + __entry->irq,
> + __entry->ifname,
> + __entry->state
And here you can have:
__print_symbolic(__entry->state,
PHY_STATE_ENUMS )
> + )
> + );
> +
> +TRACE_EVENT(phy_state_change,
> + TP_PROTO(struct phy_device *phydev, enum phy_state old_state),
> + TP_ARGS(phydev, old_state),
> + TP_STRUCT__entry(
> + __field(int, addr)
> + __field(int, state)
> + __field(int, old_state)
> + __array(char, ifname, IFNAMSIZ)
> + ),
> + TP_fast_assign(
> + __entry->addr = phydev->mdio.addr;
> + __entry->state = phydev->state;
> + __entry->old_state = old_state;
> + if (phydev->attached_dev)
> + memcpy(__entry->ifname,
> + netdev_name(phydev->attached_dev),
> + IFNAMSIZ);
> + else
> + memset(__entry->ifname, 0, IFNAMSIZ);
> + ),
> + TP_printk("phy-%d-change ifname=%16s old_state=%d state=%d",
> + __entry->addr,
> + __entry->ifname,
> + __entry->old_state,
> + __entry->state
And do the same above for both old_state and state.
Then instead of just printing numbers and having to remember what state
goes with what number (and god forbid those enums were to change), you
would get actual names of the states, and be easier to read the trace
output.
Note, I didn't test what I wrote, there could be a bug, but it
shouldn't be too far off from what I'm trying to get at.
-- Steve
> + )
> + );
> +
> +#endif /* _TRACE_PHY_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
^ permalink raw reply
* Re: [PATCH v2] net: phy: add tracepoints
From: Steven Rostedt @ 2018-08-16 15:35 UTC (permalink / raw)
To: Tobias Waldekranz
Cc: netdev, Andrew Lunn, Florian Fainelli, David S. Miller,
Ingo Molnar, open list
In-Reply-To: <20180816152452.7834-1-tobias@waldekranz.com>
On Thu, 16 Aug 2018 17:24:48 +0200
Tobias Waldekranz <tobias@waldekranz.com> wrote:
> Two tracepoints for now:
>
> * `phy_interrupt` Pretty self-explanatory.
>
> * `phy_state_change` Whenever the PHY's state machine is run, trace
> the old and the new state.
>
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> v1 -> v2:
> * Actually include the entire patch, based on current net-next.
> * Pretty print the PHY's state according to Steven's suggestion.
Cool!
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
-- Steve
> ---
^ permalink raw reply
* [PATCH v2] net: phy: add tracepoints
From: Tobias Waldekranz @ 2018-08-16 15:24 UTC (permalink / raw)
To: netdev
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Steven Rostedt,
Ingo Molnar, GitAuthor: Tobias Waldekranz, open list
Two tracepoints for now:
* `phy_interrupt` Pretty self-explanatory.
* `phy_state_change` Whenever the PHY's state machine is run, trace
the old and the new state.
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
v1 -> v2:
* Actually include the entire patch, based on current net-next.
* Pretty print the PHY's state according to Steven's suggestion.
---
drivers/net/phy/phy.c | 8 ++-
include/trace/events/phy.h | 100 +++++++++++++++++++++++++++++++++++++
2 files changed, 107 insertions(+), 1 deletion(-)
create mode 100644 include/trace/events/phy.h
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 1ee25877c4d1..74fc7151802c 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -38,6 +38,9 @@
#include <asm/irq.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/phy.h>
+
#define PHY_STATE_STR(_state) \
case PHY_##_state: \
return __stringify(_state); \
@@ -783,6 +786,7 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
if (PHY_HALTED == phydev->state)
return IRQ_NONE; /* It can't be ours. */
+ trace_phy_interrupt(irq, phydev);
return phy_change(phydev);
}
@@ -1114,10 +1118,12 @@ void phy_state_machine(struct work_struct *work)
if (err < 0)
phy_error(phydev);
- if (old_state != phydev->state)
+ if (old_state != phydev->state) {
+ trace_phy_state_change(phydev, old_state);
phydev_dbg(phydev, "PHY state change %s -> %s\n",
phy_state_to_str(old_state),
phy_state_to_str(phydev->state));
+ }
/* Only re-schedule a PHY state machine change if we are polling the
* PHY, if PHY_IGNORE_INTERRUPT is set, then we will be moving
diff --git a/include/trace/events/phy.h b/include/trace/events/phy.h
new file mode 100644
index 000000000000..c5a0c5739d4a
--- /dev/null
+++ b/include/trace/events/phy.h
@@ -0,0 +1,100 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM phy
+
+#if !defined(_TRACE_PHY_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_PHY_H
+
+#include <linux/tracepoint.h>
+
+#define PHY_STATE_ENUMS \
+ EM( DOWN ) \
+ EM( STARTING ) \
+ EM( READY ) \
+ EM( PENDING ) \
+ EM( UP ) \
+ EM( AN ) \
+ EM( RUNNING ) \
+ EM( NOLINK ) \
+ EM( FORCING ) \
+ EM( CHANGELINK )\
+ EM( HALTED ) \
+ EMe(RESUMING)
+
+#undef EM
+#undef EMe
+
+#define EM(a) TRACE_DEFINE_ENUM( PHY_##a );
+#define EMe(a) TRACE_DEFINE_ENUM( PHY_##a );
+
+PHY_STATE_ENUMS
+
+#undef EM
+#undef EMe
+
+#define EM(a) { PHY_##a, #a },
+#define EMe(a) { PHY_##a, #a }
+
+TRACE_EVENT(phy_interrupt,
+ TP_PROTO(int irq, struct phy_device *phydev),
+ TP_ARGS(irq, phydev),
+ TP_STRUCT__entry(
+ __field(int, irq)
+ __field(int, addr)
+ __field(int, state)
+ __array(char, ifname, IFNAMSIZ)
+ ),
+ TP_fast_assign(
+ __entry->irq = irq;
+ __entry->addr = phydev->mdio.addr;
+ __entry->state = phydev->state;
+ if (phydev->attached_dev)
+ memcpy(__entry->ifname,
+ netdev_name(phydev->attached_dev),
+ IFNAMSIZ);
+ else
+ memset(__entry->ifname, 0, IFNAMSIZ);
+ ),
+ TP_printk("phy-%d-irq irq=%d ifname=%-.16s state=%s",
+ __entry->addr,
+ __entry->irq,
+ __entry->ifname,
+ __print_symbolic(__entry->state, PHY_STATE_ENUMS)
+ )
+ );
+
+TRACE_EVENT(phy_state_change,
+ TP_PROTO(struct phy_device *phydev, enum phy_state old_state),
+ TP_ARGS(phydev, old_state),
+ TP_STRUCT__entry(
+ __field(int, addr)
+ __field(int, state)
+ __field(int, old_state)
+ __array(char, ifname, IFNAMSIZ)
+ ),
+ TP_fast_assign(
+ __entry->addr = phydev->mdio.addr;
+ __entry->state = phydev->state;
+ __entry->old_state = old_state;
+ if (phydev->attached_dev)
+ memcpy(__entry->ifname,
+ netdev_name(phydev->attached_dev),
+ IFNAMSIZ);
+ else
+ memset(__entry->ifname, 0, IFNAMSIZ);
+ ),
+ TP_printk("phy-%d-change ifname=%-.16s old_state=%s state=%s",
+ __entry->addr,
+ __entry->ifname,
+ __print_symbolic(__entry->old_state, PHY_STATE_ENUMS),
+ __print_symbolic(__entry->state, PHY_STATE_ENUMS)
+ )
+ );
+
+#undef EM
+#undef EMe
+#undef PHY_STATE_ENUMS
+
+#endif /* _TRACE_PHY_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 0/2] net/sched: Add hardware specific counters to TC actions
From: Eelco Chaudron @ 2018-08-16 12:02 UTC (permalink / raw)
To: David Miller, jakub.kicinski
Cc: netdev, jhs, xiyou.wangcong, jiri, simon.horman,
Marcelo Ricardo Leitner
In-Reply-To: <20180811.120627.662252154567814394.davem@davemloft.net>
On 11 Aug 2018, at 21:06, David Miller wrote:
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Thu, 9 Aug 2018 20:26:08 -0700
>
>> It is not immediately clear why this is needed. The memory and
>> updating two sets of counters won't come for free, so perhaps a
>> stronger justification than troubleshooting is due? :S
>>
>> Netdev has counters for fallback vs forwarded traffic, so you'd know
>> that traffic hits the SW datapath, plus the rules which are in_hw
>> will
>> most likely not match as of today for flower (assuming correctness).
I strongly believe that these counters are a requirement for a mixed
software/hardware (flow) based forwarding environment. The global
counters will not help much here as you might have chosen to have
certain traffic forwarded by software.
These counters are probably the only option you have to figure out why
forwarding is not as fast as expected, and you want to blame the TC
offload NIC.
>> I'm slightly concerned about potential performance impact, would you
>> be able to share some numbers for non-trivial number of flows (100k
>> active?)?
>
> Agreed, features used for diagnostics cannot have a harmful penalty
> for
> fast path performance.
Fast path performance is not affected as these counters are not
incremented there. They are only incremented by the nic driver when they
gather their statistics from hardware.
However, the flow creation is effected, as this is where the extra
memory gets allocated. I had done some 40K flow tests before and did not
see any noticeable change in flow insertion performance. As requested by
Jakub I did it again for 100K (and threw a Netronome blade in the mix
;). I used Marcelo’s test tool,
https://github.com/marceloleitner/perf-flower.git.
Here are the numbers (time in seconds) for 10 runs in sorted order:
+-------------+----------------+
| Base_kernel | Change_applied |
+-------------+----------------+
| 5.684019 | 5.656388 |
| 5.699658 | 5.674974 |
| 5.725220 | 5.722107 |
| 5.739285 | 5.839855 |
| 5.748088 | 5.865238 |
| 5.766231 | 5.873913 |
| 5.842264 | 5.909259 |
| 5.902202 | 5.912685 |
| 5.905391 | 5.947138 |
| 6.032997 | 5.997779 |
+-------------+----------------+
I guess the deviation is in the userspace part, which is where in real
life flows get added anyway.
Let me know if more is unclear.
//Eelco
^ permalink raw reply
* Re: [PATCH stable 4.4 0/9] fix SegmentSmack in stable branch (CVE-2018-5390)
From: Michal Kubecek @ 2018-08-16 11:39 UTC (permalink / raw)
To: maowenan
Cc: dwmw2, gregkh, netdev, eric.dumazet, edumazet, davem, ycheng, jdw,
stable, Takashi Iwai
In-Reply-To: <eaed8357-e779-e3fb-56d4-04e7b2da0702@huawei.com>
On Thu, Aug 16, 2018 at 03:55:16PM +0800, maowenan wrote:
> On 2018/8/16 15:44, Michal Kubecek wrote:
> > On Thu, Aug 16, 2018 at 03:39:14PM +0800, maowenan wrote:
> >>
> >>
> >> On 2018/8/16 15:23, Michal Kubecek wrote:
> >>> On Thu, Aug 16, 2018 at 03:19:12PM +0800, maowenan wrote:
> >>>> On 2018/8/16 14:52, Michal Kubecek wrote:
> >>>>>
> >>>>> My point is that backporting all this into stable 4.4 is quite intrusive
> >>>>> so that if we can achieve similar results with a simple fix of an
> >>>>> obvious omission, it would be preferrable.
> >>>>
> >>>> There are five patches in mainline to fix this CVE, only two patches
> >>>> have no effect on stable 4.4, the important reason is 4.4 use simple
> >>>> queue but mainline use RB tree.
> >>>>
> >>>> I have tried my best to use easy way to fix this with dropping packets
> >>>> 12.5%(or other value) based on simple queue, but the result is not
> >>>> very well, so the RB tree is needed and tested result is my desire.
> >>>>
> >>>> If we only back port two patches but they don't fix the issue, I think
> >>>> they don't make any sense.
> >>>
> >>> There is an obvious omission in one of the two patches and Takashi's
> >>> patch fixes it. If his follow-up fix (applied on top of what is in
> >>> stable 4.4 now) addresses the problem, I would certainly prefer using it
> >>> over backporting the whole series.
> >>
> >> Do you mean below codes from Takashi can fix this CVE?
> >> But I have already tested like this two days ago, it is not good effect.
> >
> > IIRC what you proposed was different, you proposed to replace the "=" in
> > the other branch by "+=".
>
> No, I think you don't get what I mean, I have already tested stable 4.4,
> based on commit dc6ae4d, and change the codes like Takashi, which didn't
> contain any codes I have sent in this patch series.
I suspect you may be doing something wrong with your tests. I checked
the segmentsmack testcase and the CPU utilization on receiving side
(with sending 10 times as many packets as default) went down from ~100%
to ~3% even when comparing what is in stable 4.4 now against older 4.4
kernel.
This is actually not surprising. the testcase only sends 1-byte segments
starting at even offsets so that receiver never gets two adjacent
segments and the "range_truesize != head->truesize" condition would
never be satisfied, whether you fix the backport or not.
Where the missing "range_truesize += skb->truesize" makes a difference
is in the case when there are some adjacent out of order segments, e.g.
if you omitted 1:10 and then sent 10:11, 11:12, 12:13, 13:14, ...
Then the missing update of range_truesize would prevent collapsing the
queue until the total length of the range would exceed the value of
SKB_WITH_OVERHEAD(SK_MEM_QUANTUM) (i.e. a bit less than 4 KB).
Michal Kubecek
^ permalink raw reply
* Re: [PATCH net-next 02/10] dt-bindings: net: ocelot: remove hsio from the list of register address spaces
From: Quentin Schulz @ 2018-08-16 14:25 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Rob Herring, ralf, paul.burton, jhogan, mark.rutland, davem,
kishon, andrew, f.fainelli, linux-mips, devicetree, linux-kernel,
netdev, allan.nielsen, thomas.petazzoni
In-Reply-To: <20180814124135.GK943@piout.net>
[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]
Hi Alexandre,
On Tue, Aug 14, 2018 at 02:41:35PM +0200, Alexandre Belloni wrote:
> On 14/08/2018 08:49:53+0200, Quentin Schulz wrote:
> > Understood but it's an intermediate patch. Later (patch 8), the SerDes
> > muxing "controller" is added as a child to this node. There most likely
> > will be some others in the future (temperature sensor for example).
> >
> > Furthermore, there's already a simple-mfd without children in this file:
> > https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/mips/mscc.txt#L19
> >
> > How should we handle this case?
> >
>
> There were child nodes in previous version of the binding. You can
> remove simple-mfd now. The useful registers that are not used by any
> drivers are gpr and chipid.
>
And what about the use case I'm facing? I've got child nodes defined in
it but with a later patch (but they actually haven't made it to the
DT binding documentation, so that's for the next version of the patch
series).
OK, for removing simple-mfd from CPU chip regs documentation.
Thanks,
Quentin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] net: macb: Fix regression breaking non-MDIO fixed-link PHYs
From: Andrew Lunn @ 2018-08-16 14:24 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: Uwe Kleine-König, David S. Miller, Nicolas Ferre, netdev,
mdf, stable, kernel, Brad Mouring, Florian Fainelli
In-Reply-To: <0c96cf1e-9126-71ca-802f-92b3d3ba59f7@pengutronix.de>
On Thu, Aug 16, 2018 at 08:54:40AM +0200, Ahmad Fatoum wrote:
> On 08/15/2018 04:32 AM, Andrew Lunn wrote:
> > Ahmed, where is the device tree for the EVB-KSZ9477?
>
> I've attached it [1]. It's still work-in-progress (DSA doesn't work yet for example), but Ethernet is usable with Linux v4.18 and my patch applied.
>
Thanks.
So the problem is, macb does not put phy DT nodes inside an mdio
subnode. It places them directly in the MAC node. So
of_mdiobus_register() is being called with the MAC
np. of_mdiobus_register() then looks for children of the MAC node,
assuming they are phys. But when you have a fixed phy node, it is not
a phy, it does not have a reg property, and you get these warnings.
There are cases when you need both fixed-phy and a mdio bus. e.g. a
DSA switch hanging off MDIO.
So we have a few things here...
1) A regression. We should find a fix for that. Maybe we should
special case a child node called 'fixed-link' in
of_mdiobus_register(). I would suggest adding a single warning if
such node is found.
2) Missing functionality. Add support for an mdio container node.
node = of_get_child_by_name(np, "mdio");
if (node)
err = of_mdiobus_register(bp->mii_bus, node);
else
err = of_mdiobus_register(bp->mii_bus, np);
3) Modify the existing dts files to make use of this container.
Because of backwards compatibility, we cannot force the use of it,
but we can encourage it.
Andrew
^ permalink raw reply
* Re: [PATCH v3] net/mlx5e: Delete unneeded function argument
From: Leon Romanovsky @ 2018-08-16 11:26 UTC (permalink / raw)
To: Yuval Shaia; +Cc: saeedm, davem, netdev, linux-rdma
In-Reply-To: <20180816090220.4537-1-yuval.shaia@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 534 bytes --]
On Thu, Aug 16, 2018 at 12:02:20PM +0300, Yuval Shaia wrote:
> priv argument is not used by the function, delete it.
>
> Fixes: a89842811ea98 ("net/mlx5e: Merge per priority stats groups")
> Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
> ---
> v1 -> v2:
> * Remove blank line as pointed by Leon.
>
> v2 -> v3:
> * Change prefix to mlx5e
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_stats.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Thanks Yuval,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v2] net/mlx5: Delete unneeded function argument
From: Gal Pressman @ 2018-08-16 10:31 UTC (permalink / raw)
To: Yuval Shaia; +Cc: saeedm, leon, davem, netdev, linux-rdma
In-Reply-To: <20180816090006.GB4318@lap1>
On 16-Aug-18 12:00, Yuval Shaia wrote:
> On Thu, Aug 16, 2018 at 09:41:34AM +0300, Gal Pressman wrote:
>> On 15-Aug-18 18:08, Yuval Shaia wrote:
>>> priv argument is not used by the function, delete it.
>>>
>>> Fixes: a89842811ea98 ("net/mlx5e: Merge per priority stats groups")
>>> Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
>>
>> nit: prefix should be net/mlx5e.
>
> 1. Will do.
> 2. Why? (asking since dir name is mlx5)
Core related changes use net/mlx5 prefix, Ethernet related changes (en_* files) use net/mlx5e prefix.
Same thought goes into function names prefix (mlx5_*/mlx5e_*).
>
>>
>>> ---
>>> v1 -> v2:
>>> * Remove blank line as pointed by Leon.
>>> ---
>>> drivers/net/ethernet/mellanox/mlx5/core/en_stats.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>>> index 1646859974ce..4c4d779dafa8 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>>> @@ -813,7 +813,7 @@ static const struct counter_desc pport_per_prio_traffic_stats_desc[] = {
>>>
>>> #define NUM_PPORT_PER_PRIO_TRAFFIC_COUNTERS ARRAY_SIZE(pport_per_prio_traffic_stats_desc)
>>>
>>> -static int mlx5e_grp_per_prio_traffic_get_num_stats(struct mlx5e_priv *priv)
>>> +static int mlx5e_grp_per_prio_traffic_get_num_stats(void)
>>> {
>>> return NUM_PPORT_PER_PRIO_TRAFFIC_COUNTERS * NUM_PPORT_PRIO;
>>> }
>>> @@ -971,7 +971,7 @@ static int mlx5e_grp_per_prio_pfc_fill_stats(struct mlx5e_priv *priv,
>>>
>>> static int mlx5e_grp_per_prio_get_num_stats(struct mlx5e_priv *priv)
>>> {
>>> - return mlx5e_grp_per_prio_traffic_get_num_stats(priv) +
>>> + return mlx5e_grp_per_prio_traffic_get_num_stats() +
>>> mlx5e_grp_per_prio_pfc_get_num_stats(priv);
>>> }
>>>
>>>
>>
^ permalink raw reply
* [iproute PATCH 04/10] man: devlink.8: Document -verbose option
From: Phil Sutter @ 2018-08-16 10:27 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20180816102802.31782-1-phil@nwl.cc>
This was the only bit missing in comparison to devlink help text.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
man/man8/devlink.8 | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/man/man8/devlink.8 b/man/man8/devlink.8
index ac61b6add7fba..360031f77c165 100644
--- a/man/man8/devlink.8
+++ b/man/man8/devlink.8
@@ -47,6 +47,10 @@ Generate JSON output.
.BR "\-p" , " --pretty"
When combined with -j generate a pretty JSON output.
+.TP
+.BR "\-v" , " --verbose"
+Turn on verbose output.
+
.SS
.I OBJECT
--
2.18.0
^ permalink raw reply related
* [iproute PATCH 00/10] Review help texts and man pages
From: Phil Sutter @ 2018-08-16 10:27 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
This series fixes a number of issues identified by an automated scan
over man pages and help texts.
Phil Sutter (10):
man: bridge.8: Document -oneline option
bridge: trivial: Make help text consistent
devlink: trivial: Make help text consistent
man: devlink.8: Document -verbose option
genl: Fix help text
man: ifstat.8: Document --json and --pretty options
ip: Add missing -M flag to help text
man: rtacct.8: Fix nstat options
rtmon: List options in help text
man: ss.8: Describe --events option
bridge/bridge.c | 2 +-
devlink/devlink.c | 2 +-
genl/genl.c | 4 ++--
ip/ip.c | 2 +-
ip/rtmon.c | 4 +++-
man/man8/bridge.8 | 15 ++++++++++++++-
man/man8/devlink.8 | 4 ++++
man/man8/ifstat.8 | 8 ++++++++
man/man8/rtacct.8 | 14 +++++++++-----
man/man8/ss.8 | 3 +++
10 files changed, 46 insertions(+), 12 deletions(-)
--
2.18.0
^ permalink raw reply
* [iproute PATCH 03/10] devlink: trivial: Make help text consistent
From: Phil Sutter @ 2018-08-16 10:27 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20180816102802.31782-1-phil@nwl.cc>
Typically the part of the flag in brackets completes the leading part
instead of repeating it.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
devlink/devlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 784bb84bf7ad6..519ee2577cc4c 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -5426,7 +5426,7 @@ static void help(void)
pr_err("Usage: devlink [ OPTIONS ] OBJECT { COMMAND | help }\n"
" devlink [ -f[orce] ] -b[atch] filename\n"
"where OBJECT := { dev | port | sb | monitor | dpipe | resource | region }\n"
- " OPTIONS := { -V[ersion] | -n[no-nice-names] | -j[json] | -p[pretty] | -v[verbose] }\n");
+ " OPTIONS := { -V[ersion] | -n[o-nice-names] | -j[son] | -p[retty] | -v[erbose] }\n");
}
static int dl_cmd(struct dl *dl, int argc, char **argv)
--
2.18.0
^ permalink raw reply related
* [iproute PATCH 10/10] man: ss.8: Describe --events option
From: Phil Sutter @ 2018-08-16 10:28 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20180816102802.31782-1-phil@nwl.cc>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
man/man8/ss.8 | 3 +++
1 file changed, 3 insertions(+)
diff --git a/man/man8/ss.8 b/man/man8/ss.8
index 28033d8f01dda..7a6572b173648 100644
--- a/man/man8/ss.8
+++ b/man/man8/ss.8
@@ -242,6 +242,9 @@ Print summary statistics. This option does not parse socket lists obtaining
summary from various sources. It is useful when amount of sockets is so huge
that parsing /proc/net/tcp is painful.
.TP
+.B \-E, \-\-events
+Continually display sockets as they are destroyed
+.TP
.B \-Z, \-\-context
As the
.B \-p
--
2.18.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox