* Re: [iproute PATCH v2 4/4] lib: Enable colored output only for TTYs
From: David Ahern @ 2018-08-15 16:24 UTC (permalink / raw)
To: Phil Sutter, Stephen Hemminger; +Cc: netdev, Till Maas
In-Reply-To: <20180815162127.21477-5-phil@nwl.cc>
On 8/15/18 10:21 AM, Phil Sutter wrote:
> Add an additional prerequisite to check_enable_color() to make sure
> stdout actually points to an open TTY device. Otherwise calls like
>
> | ip -color a s >/tmp/foo
>
> will print color escape sequences into that file. Allow to override this
> check by specifying '-color' flag more than once.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since v1:
> - Allow to override isatty() check by specifying '-color' flag more than
> once.
That adds overhead to my workflow where I almost always have to pipe the
output of ip to a pager.
^ permalink raw reply
* [iproute PATCH v2 0/4] A bunch of fixes regarding colored output
From: Phil Sutter @ 2018-08-15 16:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Till Maas, David Ahern
This series contains fixes for conditionally colored output in patches 1
and 2. Patch 3 merges the common conditionals from ip, tc and bridge
tools. Patch 4 then adds a further restriction to colored output to
prevent garbled output when redirecting into a file.
Changes since v1:
- Adjusted last patch according to feedback. Details given in changelog
of that patch.
Phil Sutter (4):
tc: Fix typo in check for colored output
bridge: Fix check for colored output
Merge common code for conditionally colored output
lib: Enable colored output only for TTYs
bridge/bridge.c | 5 ++---
include/color.h | 1 +
ip/ip.c | 3 +--
lib/color.c | 13 +++++++++++++
man/man8/bridge.8 | 5 ++++-
man/man8/ip.8 | 5 ++++-
man/man8/tc.8 | 5 ++++-
tc/tc.c | 3 +--
8 files changed, 30 insertions(+), 10 deletions(-)
--
2.18.0
^ permalink raw reply
* [iproute PATCH 3/4] Merge common code for conditionally colored output
From: Phil Sutter @ 2018-08-15 16:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Till Maas, David Ahern
In-Reply-To: <20180815162127.21477-1-phil@nwl.cc>
Instead of calling enable_color() conditionally with identical check in
three places, introduce check_enable_color() which does it in one place.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
bridge/bridge.c | 3 +--
include/color.h | 1 +
ip/ip.c | 3 +--
lib/color.c | 9 +++++++++
tc/tc.c | 3 +--
5 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/bridge/bridge.c b/bridge/bridge.c
index 289a157d37f03..451d684e0bcfd 100644
--- a/bridge/bridge.c
+++ b/bridge/bridge.c
@@ -200,8 +200,7 @@ main(int argc, char **argv)
_SL_ = oneline ? "\\" : "\n";
- if (color && !json)
- enable_color();
+ check_enable_color(color, json);
if (batch_file)
return batch(batch_file);
diff --git a/include/color.h b/include/color.h
index c80359d3e2e95..4f2c918db7e43 100644
--- a/include/color.h
+++ b/include/color.h
@@ -13,6 +13,7 @@ enum color_attr {
};
void enable_color(void);
+int check_enable_color(int color, int json);
void set_color_palette(void);
int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...);
enum color_attr ifa_family_color(__u8 ifa_family);
diff --git a/ip/ip.c b/ip/ip.c
index 71d5170c0cc23..38eac5ec1e17d 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -304,8 +304,7 @@ int main(int argc, char **argv)
_SL_ = oneline ? "\\" : "\n";
- if (color && !json)
- enable_color();
+ check_enable_color(color, json);
if (batch_file)
return batch(batch_file);
diff --git a/lib/color.c b/lib/color.c
index da1f516cb2492..edf96e5c6ecd7 100644
--- a/lib/color.c
+++ b/lib/color.c
@@ -77,6 +77,15 @@ void enable_color(void)
set_color_palette();
}
+int check_enable_color(int color, int json)
+{
+ if (color && !json) {
+ enable_color();
+ return 0;
+ }
+ return 1;
+}
+
void set_color_palette(void)
{
char *p = getenv("COLORFGBG");
diff --git a/tc/tc.c b/tc/tc.c
index 3bb893756f40e..e775550174473 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -515,8 +515,7 @@ int main(int argc, char **argv)
_SL_ = oneline ? "\\" : "\n";
- if (color && !json)
- enable_color();
+ check_enable_color(color, json);
if (batch_file)
return batch(batch_file);
--
2.18.0
^ permalink raw reply related
* [iproute PATCH v2 4/4] lib: Enable colored output only for TTYs
From: Phil Sutter @ 2018-08-15 16:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Till Maas, David Ahern
In-Reply-To: <20180815162127.21477-1-phil@nwl.cc>
Add an additional prerequisite to check_enable_color() to make sure
stdout actually points to an open TTY device. Otherwise calls like
| ip -color a s >/tmp/foo
will print color escape sequences into that file. Allow to override this
check by specifying '-color' flag more than once.
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.
---
lib/color.c | 6 +++++-
man/man8/bridge.8 | 5 ++++-
man/man8/ip.8 | 5 ++++-
man/man8/tc.8 | 5 ++++-
4 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/lib/color.c b/lib/color.c
index edf96e5c6ecd7..3a66d8ccb4b00 100644
--- a/lib/color.c
+++ b/lib/color.c
@@ -3,6 +3,7 @@
#include <stdarg.h>
#include <stdlib.h>
#include <string.h>
+#include <unistd.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <linux/if.h>
@@ -79,7 +80,10 @@ void enable_color(void)
int check_enable_color(int color, int json)
{
- if (color && !json) {
+ if (json || !color)
+ return 1;
+
+ if (color > 1 || isatty(fileno(stdout))) {
enable_color();
return 0;
}
diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index e7f7148315e19..b865e8b6cd771 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -171,7 +171,10 @@ return code will be non zero.
.TP
.BR "\-c" , " -color"
-Use color output.
+Use color output if stdout is a terminal. Specify twice to enable color output
+irrespective of stdout state. This flag is ignored if
+.B \-json
+is also given.
.TP
.BR "\-j", " \-json"
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 0087d18b74706..c5484bbc11483 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -188,7 +188,10 @@ executes specified command over all objects, it depends if command supports this
.TP
.BR "\-c" , " -color"
-Use color output.
+Use color output if stdout is a terminal. Specify twice to enable color output
+irrespective of stdout state. This flag is ignored if
+.B \-json
+is also given.
.TP
.BR "\-t" , " \-timestamp"
diff --git a/man/man8/tc.8 b/man/man8/tc.8
index 840880fbdba63..cbe7ac1847bbb 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -730,7 +730,10 @@ option.
.TP
.BR "\ -color"
-Use color output.
+Use color output if stdout is a terminal. Specify twice to enable color output
+irrespective of stdout state. This flag is ignored if
+.B \-json
+is also given.
.TP
.BR "\-j", " \-json"
--
2.18.0
^ permalink raw reply related
* [iproute PATCH 2/4] bridge: Fix check for colored output
From: Phil Sutter @ 2018-08-15 16:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Till Maas, David Ahern
In-Reply-To: <20180815162127.21477-1-phil@nwl.cc>
There is no point in calling enable_color() conditionally if it was
already called for each time '-color' flag was parsed. Align the
algorithm with that in ip and tc by actually making use of 'color'
variable.
Fixes: e9625d6aead11 ("Merge branch 'iproute2-master' into iproute2-next")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
bridge/bridge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bridge/bridge.c b/bridge/bridge.c
index 7fcfe1116f6e5..289a157d37f03 100644
--- a/bridge/bridge.c
+++ b/bridge/bridge.c
@@ -174,7 +174,7 @@ main(int argc, char **argv)
if (netns_switch(argv[1]))
exit(-1);
} else if (matches(opt, "-color") == 0) {
- enable_color();
+ ++color;
} else if (matches(opt, "-compressvlans") == 0) {
++compress_vlans;
} else if (matches(opt, "-force") == 0) {
--
2.18.0
^ permalink raw reply related
* [iproute PATCH 1/4] tc: Fix typo in check for colored output
From: Phil Sutter @ 2018-08-15 16:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Till Maas, David Ahern
In-Reply-To: <20180815162127.21477-1-phil@nwl.cc>
The check used binary instead of boolean AND, which means colored output
was enabled only if the number of specified '-color' flags was odd.
Fixes: 2d165c0811058 ("tc: implement color output")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
tc/tc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/tc.c b/tc/tc.c
index 3bb5910ffac52..3bb893756f40e 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -515,7 +515,7 @@ int main(int argc, char **argv)
_SL_ = oneline ? "\\" : "\n";
- if (color & !json)
+ if (color && !json)
enable_color();
if (batch_file)
--
2.18.0
^ permalink raw reply related
* [PATCH] net: dsa: add support for ksz9897 ethernet switch
From: Lad Prabhakar @ 2018-08-15 15:51 UTC (permalink / raw)
To: Woojung Huh, Microchip Linux Driver Support, Andrew Lunn,
Vivien Didelot, Florian Fainelli
Cc: netdev, devicetree, Lad, Prabhakar
From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
ksz9477 is superset of ksz9xx series, driver just works
out of the box for ksz9897 chip with this patch.
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
Documentation/devicetree/bindings/net/dsa/ksz.txt | 4 +++-
drivers/net/dsa/microchip/ksz_common.c | 9 +++++++++
drivers/net/dsa/microchip/ksz_spi.c | 1 +
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt b/Documentation/devicetree/bindings/net/dsa/ksz.txt
index a700943..ac145b8 100644
--- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
+++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
@@ -4,7 +4,9 @@ Microchip KSZ Series Ethernet switches
Required properties:
- compatible: For external switch chips, compatible string must be exactly one
- of: "microchip,ksz9477"
+ of the following:
+ - "microchip,ksz9477"
+ - "microchip,ksz9897"
See Documentation/devicetree/bindings/net/dsa/dsa.txt for a list of additional
required and optional properties.
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 7210c49..54e0ca6 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1102,6 +1102,15 @@ static const struct ksz_chip_data ksz_switch_chips[] = {
.cpu_ports = 0x7F, /* can be configured as cpu port */
.port_cnt = 7, /* total physical port count */
},
+ {
+ .chip_id = 0x00989700,
+ .dev_name = "KSZ9897",
+ .num_vlans = 4096,
+ .num_alus = 4096,
+ .num_statics = 16,
+ .cpu_ports = 0x7F, /* can be configured as cpu port */
+ .port_cnt = 7, /* total physical port count */
+ },
};
static int ksz_switch_init(struct ksz_device *dev)
diff --git a/drivers/net/dsa/microchip/ksz_spi.c b/drivers/net/dsa/microchip/ksz_spi.c
index c519469..8c1778b 100644
--- a/drivers/net/dsa/microchip/ksz_spi.c
+++ b/drivers/net/dsa/microchip/ksz_spi.c
@@ -195,6 +195,7 @@ static int ksz_spi_remove(struct spi_device *spi)
static const struct of_device_id ksz_dt_ids[] = {
{ .compatible = "microchip,ksz9477" },
+ { .compatible = "microchip,ksz9897" },
{},
};
MODULE_DEVICE_TABLE(of, ksz_dt_ids);
--
2.7.4
^ permalink raw reply related
* Re: [bpf PATCH] samples/bpf: all XDP samples should unload xdp/bpf prog on SIGTERM
From: Y Song @ 2018-08-15 15:47 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, jhsiao
In-Reply-To: <153434503438.22833.15753974916276608956.stgit@firesoul>
On Wed, Aug 15, 2018 at 7:57 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> It is common XDP practice to unload/deattach the XDP bpf program,
> when the XDP sample program is Ctrl-C interrupted (SIGINT) or
> killed (SIGTERM).
>
> The samples/bpf programs xdp_redirect_cpu and xdp_rxq_info,
> forgot to trap signal SIGTERM (which is the default signal used
> by the kill command).
>
> This was discovered by Red Hat QA, which automated scripts depend
> on killing the XDP sample program after a timeout period.
>
> Fixes: fad3917e361b ("samples/bpf: add cpumap sample program xdp_redirect_cpu")
> Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info")
> Reported-by: Jean-Tsung Hsiao <jhsiao@redhat.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> samples/bpf/xdp_redirect_cpu_user.c | 3 ++-
> samples/bpf/xdp_rxq_info_user.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
^ permalink raw reply
* Re: [PATCH stable 4.4 0/9] fix SegmentSmack (CVE-2018-5390)
From: Greg KH @ 2018-08-15 15:41 UTC (permalink / raw)
To: Mao Wenan; +Cc: dwmw2, netdev, eric.dumazet, edumazet, davem, ycheng, jdw
In-Reply-To: <20180815132432.GE31330@kroah.com>
On Wed, Aug 15, 2018 at 03:24:32PM +0200, Greg KH wrote:
> On Wed, Aug 15, 2018 at 09:20:59PM +0800, Mao Wenan wrote:
> > There are five patches to fix CVE-2018-5390 in latest mainline
> > branch, but only two patches exist in stable 4.4 and 3.18:
> > dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
> > 5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible
> > but I have tested with these patches, and found the cpu usage was very high.
> > test results:
> > with fix patch： 78.2% ksoftirqd
> > no fix patch： 90% ksoftirqd
> >
> > After analysing the codes of stable 4.4, and debuging the
> > system, the search of ofo_queue(tcp ofo using a simple queue) cost more cycles.
> > So I think only two patches can't fix the CVE-2018-5390.
> > So I try to backport "tcp: use an RB tree for ooo receive queue" using RB tree
> > instead of simple queue, then backport Eric Dumazet 5 fixed patches in mainline,
> > good news is that ksoftirqd is turn to about 20%, which is the same with mainline now.
>
> Thanks for doing this work, I had some questions on the individual
> patches. Can you address them and resend?
Also, always cc: the stable@vger list when sending stable patches so
that others can review and comment on them.
thanks,
greg k-h
^ permalink raw reply
* RE: [iproute PATCH 4/4] lib: Enable colored output only for TTYs
From: David Laight @ 2018-08-15 15:34 UTC (permalink / raw)
To: 'Stephen Hemminger', David Ahern
Cc: Phil Sutter, netdev@vger.kernel.org, Till Maas
In-Reply-To: <20180815080406.7110bd76@xeon-e3>
From: Stephen Hemminger
> Sent: 15 August 2018 16:04
...
> > This also disables color sequence when the output is piped to a pager
> > such as less which with the -R argument can handle it just fine.
> >
> > ie., the user needs to remove the color arg when that output is not wanted.
>
> If you are going to change the color enabling, why not make it compatible
> with what ls does?
Indeed - otherwise it is very hard to debug the colour escape sequences.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* RE: [PATCH] net: lan743x_ptp: convert to ktime_get_clocktai_ts64
From: Bryan.Whitehead @ 2018-08-15 18:03 UTC (permalink / raw)
To: arnd, UNGLinuxDriver; +Cc: davem, yuehaibing, netdev, linux-kernel
In-Reply-To: <20180815175040.3736548-1-arnd@arndb.de>
>
> Question: this is the only ptp driver that sets the hardware time to the
> current system time in TAI. Why does it do that?
This is done when the driver starts up after reset. Otherwise the clock is off by 48 years.
It seemed to me that the system time was the most appropriate clock to sync to.
If my reasoning is incorrect, please enlighten me.
Thanks,
Bryan
^ permalink raw reply
* [PATCH v2] net/mlx5: Delete unneeded function argument
From: Yuval Shaia @ 2018-08-15 15:08 UTC (permalink / raw)
To: saeedm, leon, davem, netdev, linux-rdma; +Cc: Yuval Shaia
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.
---
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);
}
--
2.17.1
^ permalink raw reply related
* Re: [iproute PATCH 4/4] lib: Enable colored output only for TTYs
From: Stephen Hemminger @ 2018-08-15 15:04 UTC (permalink / raw)
To: David Ahern; +Cc: Phil Sutter, netdev, Till Maas
In-Reply-To: <8f48ba0a-ba36-67f8-8dee-f0d86b72e8e0@gmail.com>
On Wed, 15 Aug 2018 08:40:20 -0600
David Ahern <dsahern@gmail.com> wrote:
> On 8/15/18 3:06 AM, Phil Sutter wrote:
> > Add an additional prerequisite to check_enable_color() to make sure
> > stdout actually points to an open TTY device. Otherwise calls like
> >
> > | ip -color a s >/tmp/foo
> >
> > will print color escape sequences into that file.
> >
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> > lib/color.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/color.c b/lib/color.c
> > index edf96e5c6ecd7..500ba09682697 100644
> > --- a/lib/color.c
> > +++ b/lib/color.c
> > @@ -3,6 +3,7 @@
> > #include <stdarg.h>
> > #include <stdlib.h>
> > #include <string.h>
> > +#include <unistd.h>
> > #include <sys/socket.h>
> > #include <sys/types.h>
> > #include <linux/if.h>
> > @@ -79,7 +80,7 @@ void enable_color(void)
> >
> > int check_enable_color(int color, int json)
> > {
> > - if (color && !json) {
> > + if (color && !json && isatty(fileno(stdout))) {
> > enable_color();
> > return 0;
> > }
> >
>
> This also disables color sequence when the output is piped to a pager
> such as less which with the -R argument can handle it just fine.
>
> ie., the user needs to remove the color arg when that output is not wanted.
If you are going to change the color enabling, why not make it compatible
with what ls does?
From man ls(1) (and grep)
--color[=WHEN]
colorize the output; WHEN can be 'always' (default if omitted),
'auto', or 'never'; more info below
...
Using color to distinguish file types is disabled both by default and
with --color=never. With --color=auto, ls emits color codes only when
standard output is connected to a terminal. The LS_COLORS environment
variable can change the settings. Use the dircolors command to set it.
Make -c be same as --color=always to keep compatiablity
^ permalink raw reply
* [PATCH] net: lan743x_ptp: convert to ktime_get_clocktai_ts64
From: Arnd Bergmann @ 2018-08-15 17:49 UTC (permalink / raw)
To: Bryan Whitehead, Microchip Linux Driver Support
Cc: Arnd Bergmann, David S. Miller, Yue Haibing, netdev, linux-kernel
timekeeping_clocktai64() has been renamed to ktime_get_clocktai_ts64()
for consistency with the other ktime_get_* access functions.
Rename the new caller that has come up as well.
Question: this is the only ptp driver that sets the hardware time
to the current system time in TAI. Why does it do that?
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/microchip/lan743x_ptp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c
index 029a2af90d5e..0e851fa3e0cc 100644
--- a/drivers/net/ethernet/microchip/lan743x_ptp.c
+++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
@@ -832,8 +832,7 @@ static void lan743x_ptp_sync_to_system_clock(struct lan743x_adapter *adapter)
{
struct timespec64 ts;
- memset(&ts, 0, sizeof(ts));
- timekeeping_clocktai64(&ts);
+ ktime_get_clocktai_ts64(&ts);
lan743x_ptp_clock_set(adapter, ts.tv_sec, ts.tv_nsec, 0);
}
--
2.18.0
^ permalink raw reply related
* [bpf PATCH] samples/bpf: all XDP samples should unload xdp/bpf prog on SIGTERM
From: Jesper Dangaard Brouer @ 2018-08-15 14:57 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, Jesper Dangaard Brouer
Cc: netdev, jhsiao
It is common XDP practice to unload/deattach the XDP bpf program,
when the XDP sample program is Ctrl-C interrupted (SIGINT) or
killed (SIGTERM).
The samples/bpf programs xdp_redirect_cpu and xdp_rxq_info,
forgot to trap signal SIGTERM (which is the default signal used
by the kill command).
This was discovered by Red Hat QA, which automated scripts depend
on killing the XDP sample program after a timeout period.
Fixes: fad3917e361b ("samples/bpf: add cpumap sample program xdp_redirect_cpu")
Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info")
Reported-by: Jean-Tsung Hsiao <jhsiao@redhat.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
samples/bpf/xdp_redirect_cpu_user.c | 3 ++-
samples/bpf/xdp_rxq_info_user.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index 9a6c7e0a6dd1..2d23054aaccf 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -679,8 +679,9 @@ int main(int argc, char **argv)
return EXIT_FAIL_OPTION;
}
- /* Remove XDP program when program is interrupted */
+ /* Remove XDP program when program is interrupted or killed */
signal(SIGINT, int_exit);
+ signal(SIGTERM, int_exit);
if (bpf_set_link_xdp_fd(ifindex, prog_fd[prog_num], xdp_flags) < 0) {
fprintf(stderr, "link set xdp fd failed\n");
diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index 248a7eab9531..ef26f882f92f 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -567,8 +567,9 @@ int main(int argc, char **argv)
exit(EXIT_FAIL_BPF);
}
- /* Remove XDP program when program is interrupted */
+ /* Remove XDP program when program is interrupted or killed */
signal(SIGINT, int_exit);
+ signal(SIGTERM, int_exit);
if (bpf_set_link_xdp_fd(ifindex, prog_fd, xdp_flags) < 0) {
fprintf(stderr, "link set xdp fd failed\n");
^ permalink raw reply related
* Re: [PATCH v3 net-next] veth: Free queues on link delete
From: David Ahern @ 2018-08-15 14:46 UTC (permalink / raw)
To: Toshiaki Makita, David S. Miller; +Cc: netdev, dsahern
In-Reply-To: <1534320449-2433-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
On 8/15/18 2:07 AM, Toshiaki Makita wrote:
> David Ahern reported memory leak in veth.
>
...
> veth_rq allocated in veth_newlink() was not freed on dellink.
>
> We need to free up them after veth_close() so that any packets will not
> reference the queues afterwards. Thus free them in veth_dev_free() in
> the same way as freeing stats structure (vstats).
>
> Also move queues allocation to veth_dev_init() to be in line with stats
> allocation.
>
> Fixes: 638264dc90227 ("veth: Support per queue XDP ring")
> Reported-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
> This is a fix for a bug which exists only in net-next.
> Let me know if I should wait for -next merging into net or reopen of -next.
>
> drivers/net/veth.c | 70 +++++++++++++++++++++++++-----------------------------
> 1 file changed, 33 insertions(+), 37 deletions(-)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
Tested-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [iproute PATCH 4/4] lib: Enable colored output only for TTYs
From: David Ahern @ 2018-08-15 14:40 UTC (permalink / raw)
To: Phil Sutter, Stephen Hemminger; +Cc: netdev, Till Maas
In-Reply-To: <20180815090610.16646-5-phil@nwl.cc>
On 8/15/18 3:06 AM, Phil Sutter wrote:
> Add an additional prerequisite to check_enable_color() to make sure
> stdout actually points to an open TTY device. Otherwise calls like
>
> | ip -color a s >/tmp/foo
>
> will print color escape sequences into that file.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> lib/color.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/color.c b/lib/color.c
> index edf96e5c6ecd7..500ba09682697 100644
> --- a/lib/color.c
> +++ b/lib/color.c
> @@ -3,6 +3,7 @@
> #include <stdarg.h>
> #include <stdlib.h>
> #include <string.h>
> +#include <unistd.h>
> #include <sys/socket.h>
> #include <sys/types.h>
> #include <linux/if.h>
> @@ -79,7 +80,7 @@ void enable_color(void)
>
> int check_enable_color(int color, int json)
> {
> - if (color && !json) {
> + if (color && !json && isatty(fileno(stdout))) {
> enable_color();
> return 0;
> }
>
This also disables color sequence when the output is piped to a pager
such as less which with the -R argument can handle it just fine.
ie., the user needs to remove the color arg when that output is not wanted.
^ permalink raw reply
* Re: [RFC PATCH net-next V2 0/6] XDP rx handler
From: David Ahern @ 2018-08-15 17:17 UTC (permalink / raw)
To: Jason Wang, Jesper Dangaard Brouer
Cc: Alexei Starovoitov, netdev, linux-kernel, ast, daniel, mst
In-Reply-To: <aa9cf883-7822-70a7-5ab5-c873b69c2098@redhat.com>
On 8/14/18 6:29 PM, Jason Wang wrote:
>
>
> On 2018年08月14日 22:03, David Ahern wrote:
>> On 8/14/18 7:20 AM, Jason Wang wrote:
>>>
>>> On 2018年08月14日 18:17, Jesper Dangaard Brouer wrote:
>>>> On Tue, 14 Aug 2018 15:59:01 +0800
>>>> Jason Wang <jasowang@redhat.com> wrote:
>>>>
>>>>> On 2018年08月14日 08:32, Alexei Starovoitov wrote:
>>>>>> On Mon, Aug 13, 2018 at 11:17:24AM +0800, Jason Wang wrote:
>>>>>>> Hi:
>>>>>>>
>>>>>>> This series tries to implement XDP support for rx hanlder. This
>>>>>>> would
>>>>>>> be useful for doing native XDP on stacked device like macvlan,
>>>>>>> bridge
>>>>>>> or even bond.
>>>>>>>
>>>>>>> The idea is simple, let stacked device register a XDP rx handler.
>>>>>>> And
>>>>>>> when driver return XDP_PASS, it will call a new helper xdp_do_pass()
>>>>>>> which will try to pass XDP buff to XDP rx handler directly. XDP rx
>>>>>>> handler may then decide how to proceed, it could consume the
>>>>>>> buff, ask
>>>>>>> driver to drop the packet or ask the driver to fallback to normal
>>>>>>> skb
>>>>>>> path.
>>>>>>>
>>>>>>> A sample XDP rx handler was implemented for macvlan. And virtio-net
>>>>>>> (mergeable buffer case) was converted to call xdp_do_pass() as an
>>>>>>> example. For ease comparision, generic XDP support for rx handler
>>>>>>> was
>>>>>>> also implemented.
>>>>>>>
>>>>>>> Compared to skb mode XDP on macvlan, native XDP on macvlan
>>>>>>> (XDP_DROP)
>>>>>>> shows about 83% improvement.
>>>>>> I'm missing the motiviation for this.
>>>>>> It seems performance of such solution is ~1M packet per second.
>>>>> Notice it was measured by virtio-net which is kind of slow.
>>>>>
>>>>>> What would be a real life use case for such feature ?
>>>>> I had another run on top of 10G mlx4 and macvlan:
>>>>>
>>>>> XDP_DROP on mlx4: 14.0Mpps
>>>>> XDP_DROP on macvlan: 10.05Mpps
>>>>>
>>>>> Perf shows macvlan_hash_lookup() and indirect call to
>>>>> macvlan_handle_xdp() are the reasons for the number drop. I think the
>>>>> numbers are acceptable. And we could try more optimizations on top.
>>>>>
>>>>> So here's real life use case is trying to have an fast XDP path for rx
>>>>> handler based device:
>>>>>
>>>>> - For containers, we can run XDP for macvlan (~70% of wire speed).
>>>>> This
>>>>> allows a container specific policy.
>>>>> - For VM, we can implement macvtap XDP rx handler on top. This
>>>>> allow us
>>>>> to forward packet to VM without building skb in the setup of macvtap.
>>>>> - The idea could be used by other rx handler based device like bridge,
>>>>> we may have a XDP fast forwarding path for bridge.
>>>>>
>>>>>> Another concern is that XDP users expect to get line rate performance
>>>>>> and native XDP delivers it. 'generic XDP' is a fallback only
>>>>>> mechanism to operate on NICs that don't have native XDP yet.
>>>>> So I can replace generic XDP TX routine with a native one for macvlan.
>>>> If you simply implement ndo_xdp_xmit() for macvlan, and instead use
>>>> XDP_REDIRECT, then we are basically done.
>>> As I replied in another thread this probably not true. Its
>>> ndo_xdp_xmit() just need to call under layer device's ndo_xdp_xmit()
>>> except for the case of bridge mode.
>>>
>>>>
>>>>>> Toshiaki's veth XDP work fits XDP philosophy and allows
>>>>>> high speed networking to be done inside containers after veth.
>>>>>> It's trying to get to line rate inside container.
>>>>> This is one of the goal of this series as well. I agree veth XDP work
>>>>> looks pretty fine, but it only work for a specific setup I believe
>>>>> since
>>>>> it depends on XDP_REDIRECT which is supported by few drivers (and
>>>>> there's no VF driver support).
>>>> The XDP_REDIRECT (RX-side) is trivial to add to drivers. It is a bad
>>>> argument that only a few drivers implement this. Especially since all
>>>> drivers also need to be extended with your proposed xdp_do_pass() call.
>>>>
>>>> (rant) The thing that is delaying XDP_REDIRECT adaption in drivers, is
>>>> that it is harder to implement the TX-side, as the ndo_xdp_xmit() call
>>>> have to allocate HW TX-queue resources. If we disconnect RX and TX
>>>> side of redirect, then we can implement RX-side in an afternoon.
>>> That's exactly the point, ndo_xdp_xmit() may requires per CPU TX queues
>>> which breaks assumptions of some drivers. And since we don't disconnect
>>> RX and TX, it looks to me the partial implementation is even worse?
>>> Consider a user can redirect from mlx4 to ixgbe but not ixgbe to mlx4.
>>>
>>>>
>>>>> And in order to make it work for a end
>>>>> user, the XDP program still need logic like hash(map) lookup to
>>>>> determine the destination veth.
>>>> That _is_ the general idea behind XDP and eBPF, that we need to add
>>>> logic
>>>> that determine the destination. The kernel provides the basic
>>>> mechanisms for moving/redirecting packets fast, and someone else
>>>> builds an orchestration tool like Cilium, that adds the needed logic.
>>> Yes, so my reply is for the concern about performance. I meant anyway
>>> the hash lookup will make it not hit the wire speed.
>>>
>>>> Did you notice that we (Ahern) added bpf_fib_lookup a FIB route lookup
>>>> accessible from XDP.
>>> Yes.
>>>
>>>> For macvlan, I imagine that we could add a BPF helper that allows you
>>>> to lookup/call macvlan_hash_lookup().
>>> That's true but we still need a method to feed macvlan with XDP buff.
>>> I'm not sure if this could be treated as another kind of redirection,
>>> but ndo_xdp_xmit() could not be used for this case for sure. Compared to
>>> redirection, XDP rx handler has its own advantages:
>>>
>>> 1) Use the exist API and userspace to setup the network topology instead
>>> of inventing new tools and its own specific API. This means user can
>>> just setup macvlan (macvtap, bridge or other) as usual and simply attach
>>> XDP programs to both macvlan and its under layer device.
>>> 2) Ease the processing of complex logic, XDP can not do cloning or
>>> reference counting. We can differ those cases and let normal networking
>>> stack to deal with such packets seamlessly. I believe this is one of the
>>> advantage of XDP. This makes us to focus on the fast path and greatly
>>> simplify the codes.
>>>
>>> Like ndo_xdp_xmit(), XDP rx handler is used to feed RX handler with XDP
>>> buff. It's just another basic mechanism. Policy is still done by XDP
>>> program itself.
>>>
>> I have been looking into handling stacked devices via lookup helper
>> functions. The idea is that a program only needs to be installed on the
>> root netdev (ie., the one representing the physical port), and it can
>> use helpers to create an efficient pipeline to decide what to do with
>> the packet in the presence of stacked devices.
>>
>> For example, anyone doing pure L3 could do:
>>
>> {port, vlan} --> [ find l2dev ] --> [ find l3dev ] ...
>>
>> --> [ l3 forward lookup ] --> [ header rewrite ] --> XDP_REDIRECT
>>
>> port is the netdev associated with the ingress_ifindex in the xdp_md
>> context, vlan is the vlan in the packet or the assigned PVID if
>> relevant. From there l2dev could be a bond or bridge device for example,
>> and l3dev is the one with a network address (vlan netdev, bond netdev,
>> etc).
>
> Looks less flexible since the topology is hard coded in the XDP program
> itself and this requires all logic to be implemented in the program on
> the root netdev.
Nothing about the topology is hard coded. The idea is to mimic a
hardware pipeline and acknowledging that a port device can have an
arbitrary layers stacked on it - multiple vlan devices, bonds, macvlans, etc
>
>>
>> I have L3 forwarding working for vlan devices and bonds. I had not
>> considered macvlans specifically yet, but it should be straightforward
>> to add.
>>
>
> Yes, and all these could be done through XDP rx handler as well, and it
> can do even more with rather simple logic:
>From a forwarding perspective I suspect the rx handler approach is going
to have much more overhead (ie., higher latency per packet and hence
lower throughput) as the layers determine which one to use (e.g., is the
FIB lookup done on the port device, vlan device, or macvlan device on
the vlan device).
>
> 1 macvlan has its own namespace, and want its own bpf logic.
> 2 Ruse the exist topology information for dealing with more complex
> setup like macvlan on top of bond and team. There's no need to bpf
> program to care about topology. If you look at the code, there's even no
> need to attach XDP on each stacked device. The calling of xdp_do_pass()
> can try to pass XDP buff to upper device even if there's no XDP program
> attached to current layer.
> 3 Deliver XDP buff to userspace through macvtap.
>
> Thanks
^ permalink raw reply
* Re: [PATCH] net/mlx5: Delete unneeded function argument
From: Leon Romanovsky @ 2018-08-15 14:20 UTC (permalink / raw)
To: Yuval Shaia; +Cc: saeedm, davem, netdev, linux-rdma
In-Reply-To: <20180815135433.23834-1-yuval.shaia@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 221 bytes --]
On Wed, Aug 15, 2018 at 04:54:33PM +0300, Yuval Shaia wrote:
> priv argument is not used by the function, delete it.
>
> Fixes: a89842811ea98 ("net/mlx5e: Merge per priority stats groups")
>
No extra space here.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* [PATCH] net/mlx5: Delete unneeded function argument
From: Yuval Shaia @ 2018-08-15 13:54 UTC (permalink / raw)
To: saeedm, leon, davem, netdev, linux-rdma; +Cc: Yuval Shaia
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>
---
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);
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 1/1] NFC: Fix possible memory corruption when handling SHDLC I-Frame commands
From: Suren Baghdasaryan @ 2018-08-15 16:40 UTC (permalink / raw)
To: Dan Carpenter
Cc: Kees Cook, Security Officers, Kevin Deus, Samuel Ortiz,
David S. Miller, Allen Pais, linux-wireless, Network Development,
LKML
In-Reply-To: <20180815082956.u6grueiyshwgqt3a@mwanda>
On Wed, Aug 15, 2018 at 1:29 AM, Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, Aug 14, 2018 at 03:38:14PM -0700, Suren Baghdasaryan wrote:
>> The separate fix for the size of pipes[] array is posted here:
>> https://lkml.org/lkml/2018/8/14/1034
>> Thanks!
>>
>
> That's great! Let's add some bounds checking to nfc_hci_msg_rx_work()
> and nfc_hci_recv_from_llc() as well and then we can close the chapter on
> these bugs.
Dan, I don't think we need additional checks there. Here are the
relevant parts of the code in nfc_hci_recv_from_llc():
static void nfc_hci_recv_from_llc(struct nfc_hci_dev *hdev, struct sk_buff *skb)
{
...
packet = (struct hcp_packet *)skb->data;
...
/* it's the last fragment. Does it need re-aggregation? */
if (skb_queue_len(&hdev->rx_hcp_frags)) {
pipe = packet->header & NFC_HCI_FRAGMENT;
...
hcp_skb = nfc_alloc_recv_skb(NFC_HCI_HCP_PACKET_HEADER_LEN +
msg_len, GFP_KERNEL);
...
*skb_put(hcp_skb, NFC_HCI_HCP_PACKET_HEADER_LEN) = pipe;
...
} else {
packet->header &= NFC_HCI_FRAGMENT;
hcp_skb = skb;
}
AFAIU in both cases the pipe field in hcp_skb can't exceed 127 after
we applied NFC_HCI_FRAGMENT(0x7f) mask.
>
> regards,
> dan carpenter
>
^ permalink raw reply
* Re: [PATCH v1 2/3] zinc: Introduce minimal cryptography library
From: D. J. Bernstein @ 2018-08-15 16:28 UTC (permalink / raw)
To: Eric Biggers, Jason A. Donenfeld, Eric Biggers,
Linux Crypto Mailing List, LKML, Netdev, David Miller,
Andrew Lutomirski, Greg Kroah-Hartman, Samuel Neves, Tanja Lange,
Jean-Philippe Aumasson, Karthikeyan Bhargavan
In-Reply-To: <20180814211229.GB24575@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2707 bytes --]
Eric Biggers writes:
> I've also written a scalar ChaCha20 implementation (no NEON instructions!) that
> is 12.2 cpb on one block at a time on Cortex-A7, taking advantage of the free
> rotates; that would be useful for the single permutation used to compute
> XChaCha's subkey, and also for the ends of messages.
This is also how ends of messages are handled in the 2012 implementation
crypto_stream/salsa20/armneon6 (see "mainloop1") inside the SUPERCOP
benchmarking framework:
https://bench.cr.yp.to/supercop.html
This code is marginally different from Eric's new code because the
occasional loads and stores are scheduled for the Cortex-A8 rather than
the Cortex-A7, and because it's Salsa20 rather than ChaCha20.
The bigger picture is that there are 63 implementations of Salsa20 and
ChaCha20 in SUPERCOP from 10 authors showing various implementation
techniques, including all the techniques that have been mentioned in
this thread; and centralized benchmarks on (e.g.)
https://bench.cr.yp.to/results-stream.html#amd64-kizomba
https://bench.cr.yp.to/web-impl/amd64-kizomba-crypto_stream-salsa20.html
showing what's fastest on various platforms, using well-developed
benchmarking tools that produce repeatable, meaningful measurements.
There are also various papers explaining the main techniques.
Of course it's possible that new code will do better, especially on
platforms with different performance characteristics from the platforms
previously targeted. Contributing new implementations to SUPERCOP is
easy---which is why SUPERCOP already has thousands of implementations of
hundreds of cryptographic functions---and is a more effective way to
advertise speedups than adding code merely to (e.g.) the Linux kernel.
Infrastructure is centralized in SUPERCOP to minimize per-implementation
work. There's no risk of being rejected on the basis of cryptographic
concerns (MD5, Speck, and RSA-512 are included in the benchmarks) or
code-style concerns. Users can then decide which implementations best
meet their requirements.
"Do better" seems to be what's happened for the Cortex-A7. The best
SUPERCOP speeds (from code targeting the Cortex-A8 etc.) are 13.42
cycles/byte for 4096 bytes for ChaCha20; 12.2, 11.9, and 11.3 sound
noticeably better. The Cortex-A7 is an interesting case because it's
simultaneously (1) widely deployed---more than a billion units sold---
but (2) poorly documented. If you want to know, e.g., which instructions
can dual-issue with loads/FPU moves/..., then you won't be able to find
anything from ARM giving the answer. I've started building an automated
tool to compute the full CPU pipeline structure from benchmarks, but
this isn't ready yet.
---Dan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* [PATCH] mac80211_hwsim: require at least one channel
From: Johannes Berg @ 2018-08-15 16:19 UTC (permalink / raw)
To: linux-wireless; +Cc: netdev, linux-kernel, syzkaller-bugs, Johannes Berg
In-Reply-To: <0000000000006ebad505737b6bf4@google.com>
From: Johannes Berg <johannes.berg@intel.com>
Syzbot continues to try to create mac80211_hwsim radios, and
manages to pass parameters that are later checked with WARN_ON
in cfg80211 - catch another one in hwsim directly.
Reported-by: syzbot+2a12f11c306afe871c1f@syzkaller.appspotmail.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
drivers/net/wireless/mac80211_hwsim.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 18e819d964f1..fe1b0108f06d 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -3194,6 +3194,11 @@ static int hwsim_new_radio_nl(struct sk_buff *msg, struct genl_info *info)
if (info->attrs[HWSIM_ATTR_CHANNELS])
param.channels = nla_get_u32(info->attrs[HWSIM_ATTR_CHANNELS]);
+ if (param.channels < 1) {
+ GENL_SET_ERR_MSG(info, "must have at least one channel");
+ return -EINVAL;
+ }
+
if (param.channels > CFG80211_MAX_NUM_DIFFERENT_CHANNELS) {
GENL_SET_ERR_MSG(info, "too many channels specified");
return -EINVAL;
--
2.14.4
^ permalink raw reply related
* net/ipv4/tcp.c:2278 WARN_ON(sock_owned_by_user(sk))
From: Adam Mitchell @ 2018-08-15 13:26 UTC (permalink / raw)
To: netdev
After moving a busy mysql database to AWS (and changing distro,
kernel, etc), we see the following kernel warning every few days (on
several hosts):
WARNING: CPU: 33 PID: 75361 at net/ipv4/tcp.c:2278 tcp_close+0x40f/0x430
This is coming from the following section of tcp_close():
/* Now socket is owned by kernel and we acquire BH lock
* to finish close. No need to check for user refs.
*/
local_bh_disable();
bh_lock_sock(sk);
WARN_ON(sock_owned_by_user(sk));
TL;DR -> Any ideas what could lead to this warning? I'm thinking that
the ____fput() (and related) code above this in the stack trace should
have left this socket in a state that could be closed without
warnings. Could some other kernel thread be setting sk->sk_lock.owned
to 1 during this tcp_close()?
It appears that the socket is NOT owned by kernel, causing the
warning. This warning does not appear to break anything - however we
will eventually receive this next error (can take over a week to
happen):
IPv4: Attempt to release TCP socket in state 10 0000000039ed388a
Once this happens, mysqld will no longer accept() new clients.
Existing open sockets work fine. We assume the above warning about
"state 10" (TCP_LISTEN) is related, but we don't know why/how that
LISTEN socket is being released or how/if it's related to the above
warnings from tcp_close().
This error comes from net/ipv4/af_inet.c:144:
if (sk->sk_type == SOCK_STREAM && sk->sk_state != TCP_CLOSE) {
pr_err("Attempt to release TCP socket in state %d %p\n",
sk->sk_state, sk);
return;
}
For what it's worth, mysqld seems completely unaware that it cannot
take new clients. GDB attached to the user-mode process (pstack)
shows that it's still listening for new connections (but they will
never come up the stack). For this reason, I don't think mysqld was
part of the attempt to close that LISTEN socket.
Thread 1 (Thread 0x7fd82b8bd8c0 (LWP 5633)):
#0 0x00007fd82942ff0d in poll () from /lib64/libc.so.6
#1 0x0000000000d9c5fe in
Mysqld_socket_listener::listen_for_connection_event() ()
#2 0x0000000000798461 in mysqld_main(int, char**) ()
#3 0x00007fd82935e445 in __libc_start_main () from /lib64/libc.so.6
#4 0x000000000078c875 in _start ()
Extra details below:
Kernel is 4.16.13-1.el7.elrepo.x86_64 #1 running in AWS.
https://elixir.bootlin.com/linux/v4.16.13/source/net/ipv4/tcp.c#L2278
https://elixir.bootlin.com/linux/v4.16.13/source/net/ipv4/af_inet.c#L144
[726780.788201] WARNING: CPU: 15 PID: 52245 at net/ipv4/tcp.c:2278
tcp_close+0x40f/0x430
[726780.794947] Modules linked in: binfmt_misc nf_conntrack_netlink
nfnetlink_queue tcp_diag inet_diag isofs ip6table_mangle ip6table_raw
ip6table_nat nf_nat_ipv6 iptable_security xt_CT iptable_raw
iptable_nat nf_nat_ipv4 nf_nat iptable_mangle xt_pkttype xt_NFLOG
nfnetlink_log xt_u32 xt_multiport xt_set xt_conntrack
ip_set_hash_netport ip_set_hash_ipport ip_set_hash_net ip_set_hash_ip
ip_set nfnetlink nf_conntrack_proto_gre nf_conntrack_ipv6
nf_defrag_ipv6 ip6table_filter ip6_tables xt_LOG nf_conntrack_tftp
nf_conntrack_ftp nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack
iptable_filter zfs(PO) zunicode(PO) zavl(PO) icp(PO) sb_edac
intel_powerclamp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel
pcbc aesni_intel crypto_simd glue_helper cryptd cirrus snd_seq
zcommon(PO) ttm intel_rapl_perf snd_seq_device
[726780.848696] drm_kms_helper znvpair(PO) snd_pcm snd_timer spl(O)
drm snd soundcore syscopyarea sysfillrect pcspkr sysimgblt input_leds
fb_sys_fops i2c_piix4 ip_tables xfs libcrc32c ata_generic pata_acpi
ata_piix xen_blkfront crc32c_intel libata ena(O) serio_raw floppy
sunrpc
[726780.863916] CPU: 15 PID: 52245 Comm: mysqld Tainted: P W O
4.16.13-1.el7.elrepo.x86_64 #1
[726780.869486] Hardware name: Xen HVM domU, BIOS 4.2.amazon 08/24/2006
[726780.873756] RIP: 0010:tcp_close+0x40f/0x430
[726780.877049] RSP: 0018:ffffc90063a87dd0 EFLAGS: 00010202
[726780.880913] RAX: 0000000000000001 RBX: ffff8839cbb4b300 RCX:
0000000000000001
[726780.885708] RDX: 0000000000400001 RSI: 0000000000023540 RDI:
000000000000002b
[726780.890412] RBP: ffffc90063a87df0 R08: 0000000000000000 R09:
0000000000000101
[726780.895192] R10: 00000000000003ff R11: 0000000000002057 R12:
ffff8839cbb4b388
[726780.900029] R13: 0000000000000009 R14: ffff8839cbb4b3c8 R15:
ffff883c5ed14900
[726780.904777] FS: 00007f6acd467700(0000) GS:ffff883c8b1c0000(0000)
knlGS:0000000000000000
[726780.909964] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[726780.914078] CR2: 00007f6cd5d430c8 CR3: 0000003c7e6b6005 CR4:
00000000001606e0
[726780.918837] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[726780.923433] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[726780.927882] Call Trace:
[726780.930410] inet_release+0x42/0x70
[726780.933423] inet6_release+0x30/0x40
[726780.936439] sock_release+0x25/0x80
[726780.939421] sock_close+0x12/0x20
[726780.942342] __fput+0xea/0x220
[726780.945170] ____fput+0xe/0x10
[726780.947963] task_work_run+0x8c/0xb0
[726780.951052] exit_to_usermode_loop+0x6b/0x95
[726780.954480] do_syscall_64+0x182/0x1b0
[726780.957586] entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[726780.961288] RIP: 0033:0x7fd82b27085d
[726780.964349] RSP: 002b:00007f6acd466b50 EFLAGS: 00000293 ORIG_RAX:
0000000000000003
[726780.969280] RAX: 0000000000000000 RBX: 00007f65b2418220 RCX:
00007fd82b27085d
[726780.973987] RDX: 0000000000000003 RSI: 00007f6d5e9990c0 RDI:
00000000000002d8
[726780.978689] RBP: 00007f6acd466c00 R08: 0000000001622848 R09:
00000000000001f8
[726780.983373] R10: 0000000000000000 R11: 0000000000000293 R12:
0000000000000000
[726780.988049] R13: 00007f6d5e9990c0 R14: 0000000001d87cc0 R15:
00000000000002d8
[726780.992700] Code: ff 48 8b 43 28 31 f6 48 89 df 48 8b 40 10 e8 49
2d 4e 00 48 8b 43 30 48 8b 80 98 01 00 00 65 48 ff 80 90 01 00 00 e9
49 ff ff ff <0f> 0b e9 ab fc ff ff 48 8b 43 28 31 f6 48 89 df 48 8b 40
10 e8
[726781.004156] ---[ end trace 8525f27644ac4631 ]---
[752066.800148] IPv4: Attempt to release TCP socket in state 10 0000000039ed388a
^ permalink raw reply
* Re: [PATCH stable 4.4 4/9] tcp: use an RB tree for ooo receive queue
From: Greg KH @ 2018-08-15 13:25 UTC (permalink / raw)
To: Mao Wenan; +Cc: dwmw2, netdev, eric.dumazet, edumazet, davem, ycheng, jdw
In-Reply-To: <1534339268-111834-5-git-send-email-maowenan@huawei.com>
On Wed, Aug 15, 2018 at 09:21:03PM +0800, Mao Wenan wrote:
> From: Yaogong Wang <wygivan@google.com>
>
> Over the years, TCP BDP has increased by several orders of magnitude,
> and some people are considering to reach the 2 Gbytes limit.
>
> Even with current window scale limit of 14, ~1 Gbytes maps to ~740,000
> MSS.
>
> In presence of packet losses (or reorders), TCP stores incoming packets
> into an out of order queue, and number of skbs sitting there waiting for
> the missing packets to be received can be in the 10^5 range.
>
> Most packets are appended to the tail of this queue, and when
> packets can finally be transferred to receive queue, we scan the queue
> from its head.
>
> However, in presence of heavy losses, we might have to find an arbitrary
> point in this queue, involving a linear scan for every incoming packet,
> throwing away cpu caches.
>
> This patch converts it to a RB tree, to get bounded latencies.
>
> Yaogong wrote a preliminary patch about 2 years ago.
> Eric did the rebase, added ofo_last_skb cache, polishing and tests.
>
> Tested with network dropping between 1 and 10 % packets, with good
> success (about 30 % increase of throughput in stress tests)
>
> Next step would be to also use an RB tree for the write queue at sender
> side ;)
>
> Signed-off-by: Yaogong Wang <wygivan@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> Cc: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> Acked-By: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: root <root@localhost.localdomain>
root and commit id?
^ 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