* [PATCH net-next 6/6] vxlan: support flow dissect
From: Tom Herbert @ 2017-08-29 17:19 UTC (permalink / raw)
To: davem; +Cc: netdev, Tom Herbert
In-Reply-To: <20170829171942.8974-1-tom@quantonium.net>
Populate offload flow_dissect callback appropriately for VXLAN and
VXLAN-GPE.
---
drivers/net/vxlan.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index ae3a1da703c2..41e50de40af4 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1336,6 +1336,55 @@ static bool vxlan_ecn_decapsulate(struct vxlan_sock *vs, void *oiph,
return err <= 1;
}
+static enum flow_dissect_ret vxlan_flow_dissect(struct sock *sk,
+ const struct sk_buff *skb,
+ struct flow_dissector_key_control *key_control,
+ struct flow_dissector *flow_dissector,
+ void *target_container, void *data,
+ __be16 *p_proto, u8 *p_ip_proto, int *p_nhoff,
+ int *p_hlen, unsigned int flags)
+{
+ __be16 protocol = htons(ETH_P_TEB);
+ struct vxlanhdr *vhdr, _vhdr;
+ struct vxlan_sock *vs;
+
+ vhdr = __skb_header_pointer(skb, *p_nhoff + sizeof(struct udphdr),
+ sizeof(_vhdr), data, *p_hlen, &_vhdr);
+ if (!vhdr)
+ return FLOW_DISSECT_RET_OUT_BAD;
+
+ vs = rcu_dereference_sk_user_data(sk);
+ if (!vs)
+ return FLOW_DISSECT_RET_OUT_BAD;
+
+ if (vs->flags & VXLAN_F_GPE) {
+ struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)vhdr;
+
+ /* Need to have Next Protocol set for interfaces in GPE mode. */
+ if (gpe->version != 0 || !gpe->np_applied || gpe->oam_flag)
+ return FLOW_DISSECT_RET_CONTINUE;
+
+ switch (gpe->next_protocol) {
+ case VXLAN_GPE_NP_IPV4:
+ protocol = htons(ETH_P_IP);
+ break;
+ case VXLAN_GPE_NP_IPV6:
+ protocol = htons(ETH_P_IPV6);
+ break;
+ case VXLAN_GPE_NP_ETHERNET:
+ protocol = htons(ETH_P_TEB);
+ break;
+ default:
+ return FLOW_DISSECT_RET_CONTINUE;
+ }
+ }
+
+ *p_nhoff += sizeof(struct udphdr) + sizeof(_vhdr);
+ *p_proto = protocol;
+
+ return FLOW_DISSECT_RET_PROTO_AGAIN;
+}
+
/* Callback from net/ipv4/udp.c to receive packets */
static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
{
@@ -2864,6 +2913,7 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, bool ipv6,
tunnel_cfg.encap_destroy = NULL;
tunnel_cfg.gro_receive = vxlan_gro_receive;
tunnel_cfg.gro_complete = vxlan_gro_complete;
+ tunnel_cfg.flow_dissect = vxlan_flow_dissect;
setup_udp_tunnel_sock(net, sock, &tunnel_cfg);
--
2.11.0
^ permalink raw reply related
* Re: [patch v1 1/2] dt-bindings: net: add binding documentation for mlxsw thermal control
From: Andrew Lunn @ 2017-08-29 17:22 UTC (permalink / raw)
To: Vadim Pasternak; +Cc: robh+dt, davem, jiri, ivecera, devicetree, netdev
In-Reply-To: <1504032311-195988-2-git-send-email-vadimp@mellanox.com>
> +- compatible : "mellanox,mlxsw_minimal"
Interesting product name. Is there a mlxsw_maximal planned?
> +- reg : The I2C address of the device.
> +
> +Optional properties:
> +- cooling-phandle : phandle of the cooling device, which is to be used
> + for the zone thermal control.
> + If absent, cooling device controlled internally by
> + the ASIC may be used.
> +
> +- trips : the nodes to describe a point in the temperature
> + domain with key temperatures at which cooling is
> + recommended. Each node must contain the next values:
> + - type: the trip type. Expected values are:
> + 0 - a trip point to enable active cooling;
> + 1 - a trip point to enable passive cooling;
> + 2 - a trip point to notify emergency;
> + - temperature: unsigned integer indicating the trip
> + temperature level in millicelsius;
> + - minimum cooling state allowed within the trip node;
> + - maximum cooling state allowed within the trip node;
> +
> +Example:
> + asic_thermal: mlxsw_minimal@48 {
> + compatible = "mlxsw_minimal";
You missed the vendor part.
> + reg = <0x48>;
> + status = "disabled";
An example with it disabled?
> + cooling-phandle = <&cooling>;
> +
> + trips {
> + trip@0 {
> + trip = <0 75000 0 0>;
> + };
I don't know much about the thermal subsystem. But looking at other
example binding documents, you seem to do something different here to
other drivers. Why do you not use what seems to be the common format:
trips {
cpu_alert0: cpu-alert0 {
temperature = <90000>; /* millicelsius */
hysteresis = <2000>; /* millicelsius */
type = "active";
};
cpu_alert1: cpu-alert1 {
temperature = <100000>; /* millicelsius */
hysteresis = <2000>; /* millicelsius */
type = "passive";
};
cpu_crit: cpu-crit {
temperature = <125000>; /* millicelsius */
hysteresis = <2000>; /* millicelsius */
type = "critical";
};
};
Andrew
^ permalink raw reply
* Re: [PATCH net-next 7/7] samples/bpf: xdp_monitor tool based on tracepoints
From: Daniel Borkmann @ 2017-08-29 17:24 UTC (permalink / raw)
To: Alexei Starovoitov, Jesper Dangaard Brouer; +Cc: netdev, John Fastabend
In-Reply-To: <20170829170551.5uws25py4fcpem73@ast-mbp>
On 08/29/2017 07:05 PM, Alexei Starovoitov wrote:
> On Tue, Aug 29, 2017 at 04:38:11PM +0200, Jesper Dangaard Brouer wrote:
>> This tool xdp_monitor demonstrate how to use the different xdp_redirect
>> tracepoints xdp_redirect{,_map}{,_err} from a BPF program.
>>
>> The default mode is to only monitor the error counters, to avoid
>> affecting the per packet performance. Tracepoints comes with a base
>> overhead of 25 nanosec for an attached bpf_prog, and 48 nanosec for
>> using a full perf record (with non-matching filter). Thus, default
>> loading the --stats mode could affect the maximum performance.
>>
>> This version of the tool is very simple and count all types of errors
>> as one. It will be natural to extend this later with the different
>> types of errors that can occur, which should help users quickly
>> identify common mistakes.
>>
>> Because the TP_STRUCT was kept in sync all the tracepoints loads the
>> same BPF code. It would also be natural to extend the map version to
>> demonstrate how the map information could be used.
>>
>> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> Nice. Did you consider using libbbpf (instead of old bpf_load.c hack)
> and make full standalone tool out of it? Looks very useful.
+1 also my suggestion. ;)
> Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next] staging: irda: force to be a kernel module
From: Greg KH @ 2017-08-29 17:26 UTC (permalink / raw)
To: David Miller; +Cc: samuel, netdev, linux-kernel, devel
In-Reply-To: <20170829.093507.2166038228205751885.davem@davemloft.net>
On Tue, Aug 29, 2017 at 09:35:07AM -0700, David Miller wrote:
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Date: Tue, 29 Aug 2017 11:14:17 +0200
>
> > Now that the IRDA networking code has moved into drivers/staging/, the
> > link order is changed for when it is initialized if built into the
> > system. This can cause a crash when initializing as the netfilter core
> > hasn't been initialized yet.
> >
> > So force the IRDA code to be built as a module, preventing the crash.
> >
> > Reported-by: kernel test robot <fengguang.wu@intel.com>
> > Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
>
> I don't think this is reasonable.
>
> IRDA being built in was broken by moving it to staging, so it's a
> regression and we should find a way to fix it.
Hm, this is due to netlink coming before irda in the link order before
this patch series. I can't change the link order to put all of net/
before drivers/, which would solve this, and I don't think I can put:
obj-$(CONFIG_IRDA) += ../../drivers/staging/irda/net/
in a networking Makefile, can I? Does "../" even work in a Makefile
like that?
Any other thoughts?
> It's one thing if IRDA on it's own has deteriorated and broken in some
> ways over time due to lack of maintainence, it's another to knowingly
> do something to it that causes a regression which is what happened
> here.
It has deteriorated and is broken and does not work at all from the
reports I have gotten, Linus pointing this out to me directly due to his
involvement in irda-related dive computers. So I don't think anyone is
using this at all right now, it seems to crash when used anyway. So no
one is running this "build in" code at the moment :)
ideas?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net-next] staging: irda: force to be a kernel module
From: Greg KH @ 2017-08-29 17:31 UTC (permalink / raw)
To: David Miller; +Cc: devel, netdev, samuel, linux-kernel
In-Reply-To: <20170829172608.GA4700@kroah.com>
On Tue, Aug 29, 2017 at 07:26:08PM +0200, Greg KH wrote:
> On Tue, Aug 29, 2017 at 09:35:07AM -0700, David Miller wrote:
> > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Date: Tue, 29 Aug 2017 11:14:17 +0200
> >
> > > Now that the IRDA networking code has moved into drivers/staging/, the
> > > link order is changed for when it is initialized if built into the
> > > system. This can cause a crash when initializing as the netfilter core
> > > hasn't been initialized yet.
> > >
> > > So force the IRDA code to be built as a module, preventing the crash.
> > >
> > > Reported-by: kernel test robot <fengguang.wu@intel.com>
> > > Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
> >
> > I don't think this is reasonable.
> >
> > IRDA being built in was broken by moving it to staging, so it's a
> > regression and we should find a way to fix it.
>
> Hm, this is due to netlink coming before irda in the link order before
> this patch series. I can't change the link order to put all of net/
> before drivers/, which would solve this, and I don't think I can put:
> obj-$(CONFIG_IRDA) += ../../drivers/staging/irda/net/
> in a networking Makefile, can I? Does "../" even work in a Makefile
> like that?
Wait, I think that does work, let me go test this some more...
thanks,
greg k-h-
^ permalink raw reply
* Re: [PATCH net-next v2] bridge: fdb add and delete tracepoints
From: Roopa Prabhu @ 2017-08-29 17:36 UTC (permalink / raw)
To: Florian Fainelli
Cc: davem@davemloft.net, netdev@vger.kernel.org, Nikolay Aleksandrov,
bridge
In-Reply-To: <8737eb73-3437-5529-6f4d-7aa52c770357@gmail.com>
On Tue, Aug 29, 2017 at 9:46 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 08/28/2017 09:22 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> A few useful tracepoints to trace bridge forwarding
>> database updates.
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>
> Small nit below, but probably not a candidate for a v3
>
>> ---
>
>> +
>> + TP_printk("dev %s addr %02x:%02x:%02x:%02x:%02x:%02x vid %u nlh_flags %04x ndm_flags = %02x",
>
> Small nit, any particular reason why ndm_flags got a special treatment
> with an equal character and not the other?
>
good eyes, thats a typo. I did scan them once and removed the '=' and
missed a spot.
i will send v3.
^ permalink raw reply
* Re: [PATCH net-next] staging: irda: force to be a kernel module
From: David Miller @ 2017-08-29 17:40 UTC (permalink / raw)
To: gregkh; +Cc: samuel, netdev, linux-kernel, devel
In-Reply-To: <20170829172608.GA4700@kroah.com>
From: Greg KH <gregkh@linuxfoundation.org>
Date: Tue, 29 Aug 2017 19:26:08 +0200
> On Tue, Aug 29, 2017 at 09:35:07AM -0700, David Miller wrote:
>> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Date: Tue, 29 Aug 2017 11:14:17 +0200
>>
>> > Now that the IRDA networking code has moved into drivers/staging/, the
>> > link order is changed for when it is initialized if built into the
>> > system. This can cause a crash when initializing as the netfilter core
>> > hasn't been initialized yet.
>> >
>> > So force the IRDA code to be built as a module, preventing the crash.
>> >
>> > Reported-by: kernel test robot <fengguang.wu@intel.com>
>> > Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
>>
>> I don't think this is reasonable.
>>
>> IRDA being built in was broken by moving it to staging, so it's a
>> regression and we should find a way to fix it.
>
> Hm, this is due to netlink coming before irda in the link order before
> this patch series. I can't change the link order to put all of net/
> before drivers/, which would solve this, and I don't think I can put:
> obj-$(CONFIG_IRDA) += ../../drivers/staging/irda/net/
> in a networking Makefile, can I? Does "../" even work in a Makefile
> like that?
>
> Any other thoughts?
Change the initialization type in IRDA from subsys_init() to ...
something else?
Amazing!
^ permalink raw reply
* [PATCH v2 net-next] irda: fix link order if IRDA is built into the kernel
From: Greg KH @ 2017-08-29 17:46 UTC (permalink / raw)
To: David Miller
Cc: devel, samuel, netdev, linux-kernel, Geert Uytterhoeven,
kernel test robot
In-Reply-To: <20170829173129.GA11029@kroah.com>
When moving the IRDA code out of net/ into drivers/staging/irda/net, the
link order changes when IRDA is built into the kernel. That causes a
kernel crash at boot time as netfilter isn't initialized yet.
To fix this, build and link the irda networking code in the same exact
order that it was previously before the move.
Reported-by: kernel test robot <fengguang.wu@intel.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
---
v2 - don't force irda to be a module, make the Makefiles put irda back
where it was before in the link order.
drivers/staging/Makefile | 1 -
net/Makefile | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index fced929a0e67..1192caa94435 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -2,7 +2,6 @@
obj-y += media/
obj-y += typec/
-obj-$(CONFIG_IRDA) += irda/net/
obj-$(CONFIG_IRDA) += irda/drivers/
obj-$(CONFIG_PRISM2_USB) += wlan-ng/
obj-$(CONFIG_COMEDI) += comedi/
diff --git a/net/Makefile b/net/Makefile
index 3d3feff3643b..ddd059c3dfa4 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_NETROM) += netrom/
obj-$(CONFIG_ROSE) += rose/
obj-$(CONFIG_AX25) += ax25/
obj-$(CONFIG_CAN) += can/
+obj-$(CONFIG_IRDA) += ../drivers/staging/irda/net/
obj-$(CONFIG_BT) += bluetooth/
obj-$(CONFIG_SUNRPC) += sunrpc/
obj-$(CONFIG_AF_RXRPC) += rxrpc/
--
2.14.1
^ permalink raw reply related
* Re: [PATCH v2 net-next] irda: fix link order if IRDA is built into the kernel
From: David Miller @ 2017-08-29 17:49 UTC (permalink / raw)
To: gregkh; +Cc: devel, netdev, samuel, linux-kernel, fengguang.wu, geert
In-Reply-To: <20170829174622.GA25926@kroah.com>
From: Greg KH <gregkh@linuxfoundation.org>
Date: Tue, 29 Aug 2017 19:46:22 +0200
> When moving the IRDA code out of net/ into drivers/staging/irda/net, the
> link order changes when IRDA is built into the kernel. That causes a
> kernel crash at boot time as netfilter isn't initialized yet.
>
> To fix this, build and link the irda networking code in the same exact
> order that it was previously before the move.
>
> Reported-by: kernel test robot <fengguang.wu@intel.com>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
Greg, just change the initializer in IRDA so that it will run
after subsys_init() when built statically.
IRDA is definitely not the first pontentially statically built
thing that needs netlink up and available.
^ permalink raw reply
* Re: [PATCH net-next 0/7] XDP redirect tracepoints
From: David Miller @ 2017-08-29 17:51 UTC (permalink / raw)
To: brouer; +Cc: netdev, john.fastabend
In-Reply-To: <150401743083.16384.15778781741742858567.stgit@firesoul>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Tue, 29 Aug 2017 16:37:35 +0200
> I feel this is as far as I can take the tracepoint infrastructure to
> assist XDP monitoring.
>
> Tracepoints comes with a base overhead of 25 nanosec for an attached
> bpf_prog, and 48 nanosec for using a full perf record. This is
> problematic for the XDP use-case, but it is very convenient to use the
> existing perf infrastructure.
>
>>From a performance perspective, the real solution would be to attach
> another bpf_prog (that understand xdp_buff), but I'm not sure we want
> to introduce yet another bpf attach API for this.
>
> One thing left is to standardize the possible err return codes, to a
> limited set, to allow easier (and faster) mapping into a bpf map.
Series applied, thanks Jesper.
^ permalink raw reply
* Re: UDP sockets oddities
From: Florian Fainelli @ 2017-08-29 17:53 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: netdev, pabeni, willemb
In-Reply-To: <deb227c9-728a-bd98-f282-1478d71353a8@gmail.com>
On 08/26/2017 11:56 AM, Florian Fainelli wrote:
>
>
> On 08/26/2017 05:47 AM, Eric Dumazet wrote:
>> On Fri, 2017-08-25 at 21:19 -0700, David Miller wrote:
>>
>>> Agreed, but the ARP resolution queue really needs to scale it's backlog
>>> to the physical technology it is attached to.
>> Yes, last time (in 2011) we increased the old limit of 3 packets :/
>>
>> We probably should match sysctl_wmem_max so that a single socket
>> provider would hit its sk_sndbuf limit
Eric, do you want to post this as a formal patch? I don't think I
understand these tunables enough to provide a good commit message
anyways. Thanks!
>
> Before:
> /proc/sys/net/ipv4/neigh/eth0/unres_qlen:34
> /proc/sys/net/ipv4/neigh/eth0/unres_qlen_bytes:65536
> /proc/sys/net/ipv4/neigh/gphy/unres_qlen:34
> /proc/sys/net/ipv4/neigh/gphy/unres_qlen_bytes:65536
>
> After:
> /proc/sys/net/ipv4/neigh/eth0/unres_qlen:106
> /proc/sys/net/ipv4/neigh/eth0/unres_qlen_bytes:229376
> /proc/sys/net/ipv4/neigh/gphy/unres_qlen:106
> /proc/sys/net/ipv4/neigh/gphy/unres_qlen_bytes:229376
>
> and this does help a lot with the test case reported over an hour, only
> 2 packets lost:
>
> # perf record -a -g -e skb:kfree_skb iperf -c 192.168.1.23 -b 900M -t
> 3600 -u
> ------------------------------------------------------------
> Client connecting to 192.168.1.23, UDP port 5001
> Sending 1470 byte datagrams, IPG target: 13.07 us (kalman adjust)
> UDP buffer size: 224 KByte (default)
> ------------------------------------------------------------
> [ 4] local 192.168.1.66 port 48209 connected with 192.168.1.23 port 5001
> write failed: Invalid argument
> [ ID] Interval Transfer Bandwidth
> [ 4] 0.0-404.9 sec 4.51 GBytes 95.7 Mbits/sec
> [ 4] Sent 3294727 datagrams
> [ 4] Server Report:
> [ 4] 0.0-405.1 sec 4.51 GBytes 95.6 Mbits/sec 14.979 ms
> 2/3294728 (6.1e-05%)
>
> Thanks Eric!
>
>>
>> Something like :
>>
>> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
>> index 6b0bc0f715346a097a6df46e2ba2771359abcd23..7777dceb78107c0019fb39d5b69be1959005b78e 100644
>> --- a/Documentation/networking/ip-sysctl.txt
>> +++ b/Documentation/networking/ip-sysctl.txt
>> @@ -109,7 +109,8 @@ neigh/default/unres_qlen_bytes - INTEGER
>> queued for each unresolved address by other network layers.
>> (added in linux 3.3)
>> Setting negative value is meaningless and will return error.
>> - Default: 65536 Bytes(64KB)
>> + Default: SK_WMEM_MAX, enough to store 256 packets of medium size
>> + (less than 256 bytes per packet)
>>
>> neigh/default/unres_qlen - INTEGER
>> The maximum number of packets which may be queued for each
>> diff --git a/include/net/sock.h b/include/net/sock.h
>> index 1c2912d433e81b10f3fdc87bcfcbb091570edc03..03a362568357acc7278a318423dd3873103f90ca 100644
>> --- a/include/net/sock.h
>> +++ b/include/net/sock.h
>> @@ -2368,6 +2368,16 @@ bool sk_net_capable(const struct sock *sk, int cap);
>>
>> void sk_get_meminfo(const struct sock *sk, u32 *meminfo);
>>
>> +/* Take into consideration the size of the struct sk_buff overhead in the
>> + * determination of these values, since that is non-constant across
>> + * platforms. This makes socket queueing behavior and performance
>> + * not depend upon such differences.
>> + */
>> +#define _SK_MEM_PACKETS 256
>> +#define _SK_MEM_OVERHEAD SKB_TRUESIZE(256)
>> +#define SK_WMEM_MAX (_SK_MEM_OVERHEAD * _SK_MEM_PACKETS)
>> +#define SK_RMEM_MAX (_SK_MEM_OVERHEAD * _SK_MEM_PACKETS)
>> +
>> extern __u32 sysctl_wmem_max;
>> extern __u32 sysctl_rmem_max;
>>
>> diff --git a/net/core/sock.c b/net/core/sock.c
>> index dfdd14cac775e9bfcee0085ee32ffcd0ab28b67b..9b7b6bbb2a23e7652a1f34a305f29d49de00bc8c 100644
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>> @@ -307,16 +307,6 @@ static struct lock_class_key af_wlock_keys[AF_MAX];
>> static struct lock_class_key af_elock_keys[AF_MAX];
>> static struct lock_class_key af_kern_callback_keys[AF_MAX];
>>
>> -/* Take into consideration the size of the struct sk_buff overhead in the
>> - * determination of these values, since that is non-constant across
>> - * platforms. This makes socket queueing behavior and performance
>> - * not depend upon such differences.
>> - */
>> -#define _SK_MEM_PACKETS 256
>> -#define _SK_MEM_OVERHEAD SKB_TRUESIZE(256)
>> -#define SK_WMEM_MAX (_SK_MEM_OVERHEAD * _SK_MEM_PACKETS)
>> -#define SK_RMEM_MAX (_SK_MEM_OVERHEAD * _SK_MEM_PACKETS)
>> -
>> /* Run time adjustable parameters. */
>> __u32 sysctl_wmem_max __read_mostly = SK_WMEM_MAX;
>> EXPORT_SYMBOL(sysctl_wmem_max);
>> diff --git a/net/decnet/dn_neigh.c b/net/decnet/dn_neigh.c
>> index 21dedf6fd0f76dec22b2b3685beb89cfefea7ded..22bf0b95d6edc3c27ef3a99d27cb70a1551e3e0e 100644
>> --- a/net/decnet/dn_neigh.c
>> +++ b/net/decnet/dn_neigh.c
>> @@ -94,7 +94,7 @@ struct neigh_table dn_neigh_table = {
>> [NEIGH_VAR_BASE_REACHABLE_TIME] = 30 * HZ,
>> [NEIGH_VAR_DELAY_PROBE_TIME] = 5 * HZ,
>> [NEIGH_VAR_GC_STALETIME] = 60 * HZ,
>> - [NEIGH_VAR_QUEUE_LEN_BYTES] = 64*1024,
>> + [NEIGH_VAR_QUEUE_LEN_BYTES] = SK_WMEM_MAX,
>> [NEIGH_VAR_PROXY_QLEN] = 0,
>> [NEIGH_VAR_ANYCAST_DELAY] = 0,
>> [NEIGH_VAR_PROXY_DELAY] = 0,
>> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
>> index 8b52179ddc6e54eabf6d3c2ed0132083228680bb..7c45b8896709815c5dde5972fd57cb5c3bcb2648 100644
>> --- a/net/ipv4/arp.c
>> +++ b/net/ipv4/arp.c
>> @@ -171,7 +171,7 @@ struct neigh_table arp_tbl = {
>> [NEIGH_VAR_BASE_REACHABLE_TIME] = 30 * HZ,
>> [NEIGH_VAR_DELAY_PROBE_TIME] = 5 * HZ,
>> [NEIGH_VAR_GC_STALETIME] = 60 * HZ,
>> - [NEIGH_VAR_QUEUE_LEN_BYTES] = 64 * 1024,
>> + [NEIGH_VAR_QUEUE_LEN_BYTES] = SK_WMEM_MAX,
>> [NEIGH_VAR_PROXY_QLEN] = 64,
>> [NEIGH_VAR_ANYCAST_DELAY] = 1 * HZ,
>> [NEIGH_VAR_PROXY_DELAY] = (8 * HZ) / 10,
>> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
>> index 5e338eb89509b1df6ebd060f8bd19fcb4b86fe05..266a530414d7be4f1e7be922e465bbab46f7cbac 100644
>> --- a/net/ipv6/ndisc.c
>> +++ b/net/ipv6/ndisc.c
>> @@ -127,7 +127,7 @@ struct neigh_table nd_tbl = {
>> [NEIGH_VAR_BASE_REACHABLE_TIME] = ND_REACHABLE_TIME,
>> [NEIGH_VAR_DELAY_PROBE_TIME] = 5 * HZ,
>> [NEIGH_VAR_GC_STALETIME] = 60 * HZ,
>> - [NEIGH_VAR_QUEUE_LEN_BYTES] = 64 * 1024,
>> + [NEIGH_VAR_QUEUE_LEN_BYTES] = SK_WMEM_MAX,
>> [NEIGH_VAR_PROXY_QLEN] = 64,
>> [NEIGH_VAR_ANYCAST_DELAY] = 1 * HZ,
>> [NEIGH_VAR_PROXY_DELAY] = (8 * HZ) / 10,
>>
>>
>
--
Florian
^ permalink raw reply
* Re: [PATCH net] ipv6: do not set sk_destruct in IPV6_ADDRFORM sockopt
From: David Miller @ 2017-08-29 17:56 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, pabeni, chunwang, syzkaller
In-Reply-To: <4cfe77b4a829c0d6134b842fe2ea7c41b6b210ff.1503888301.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Mon, 28 Aug 2017 10:45:01 +0800
> ChunYu found a kernel warn_on during syzkaller fuzzing:
>
> [40226.038539] WARNING: CPU: 5 PID: 23720 at net/ipv4/af_inet.c:152 inet_sock_destruct+0x78d/0x9a0
> [40226.144849] Call Trace:
> [40226.147590] <IRQ>
> [40226.149859] dump_stack+0xe2/0x186
> [40226.176546] __warn+0x1a4/0x1e0
> [40226.180066] warn_slowpath_null+0x31/0x40
> [40226.184555] inet_sock_destruct+0x78d/0x9a0
> [40226.246355] __sk_destruct+0xfa/0x8c0
> [40226.290612] rcu_process_callbacks+0xaa0/0x18a0
> [40226.336816] __do_softirq+0x241/0x75e
> [40226.367758] irq_exit+0x1f6/0x220
> [40226.371458] smp_apic_timer_interrupt+0x7b/0xa0
> [40226.376507] apic_timer_interrupt+0x93/0xa0
>
> The warn_on happned when sk->sk_rmem_alloc wasn't 0 in inet_sock_destruct.
> As after commit f970bd9e3a06 ("udp: implement memory accounting helpers"),
> udp has changed to use udp_destruct_sock as sk_destruct where it would
> udp_rmem_release all rmem.
>
> But IPV6_ADDRFORM sockopt sets sk_destruct with inet_sock_destruct after
> changing family to PF_INET. If rmem is not 0 at that time, and there is
> no place to release rmem before calling inet_sock_destruct, the warn_on
> will be triggered.
>
> This patch is to fix it by not setting sk_destruct in IPV6_ADDRFORM sockopt
> any more. As IPV6_ADDRFORM sockopt only works for tcp and udp. TCP sock has
> already set it's sk_destruct with inet_sock_destruct and UDP has set with
> udp_destruct_sock since they're created.
>
> Fixes: f970bd9e3a06 ("udp: implement memory accounting helpers")
> Reported-by: ChunYu Wang <chunwang@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] net: stmmac: constify clk_div_table
From: David Miller @ 2017-08-29 17:56 UTC (permalink / raw)
To: arvind.yadav.cs
Cc: khilman, carlo, alexandre.torgue, peppe.cavallaro, linux-kernel,
linux-amlogic, linux-arm-kernel, netdev
In-Reply-To: <a54fe9764d287df370c16b0f0814d09d8fa24591.1503899422.git.arvind.yadav.cs@gmail.com>
From: Arvind Yadav <arvind.yadav.cs@gmail.com>
Date: Mon, 28 Aug 2017 11:22:20 +0530
> clk_div_table are not supposed to change at runtime.
> meson8b_dwmac structure is working with const clk_div_table.
> So mark the non-const structs as const.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
Applied.
^ permalink raw reply
* RE: [patch v1 1/2] dt-bindings: net: add binding documentation for mlxsw thermal control
From: Vadim Pasternak @ 2017-08-29 17:57 UTC (permalink / raw)
To: Andrew Lunn
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org,
ivecera-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20170829172254.GG8235-g2DYL2Zd6BY@public.gmane.org>
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew-g2DYL2Zd6BY@public.gmane.org]
> Sent: Tuesday, August 29, 2017 8:23 PM
> To: Vadim Pasternak <vadimp-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org;
> ivecera-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [patch v1 1/2] dt-bindings: net: add binding documentation for
> mlxsw thermal control
>
> > +- compatible : "mellanox,mlxsw_minimal"
>
> Interesting product name. Is there a mlxsw_maximal planned?
>
Hi Andrew,
Thank you very much for review.
No plans for such product. We just have fully functional drivers for different
kind of Mellanox switch devices like spectrum, switchib, switchx2. All of them
work over PCI bus. The minimal is supposed to be used for the chassis
management and we uses it at BMC side. It works over I2C bus and doesn't
depend on switch type. So it has a minaml functionality, so name "minimal".
> > +- reg : The I2C address of the device.
> > +
> > +Optional properties:
> > +- cooling-phandle : phandle of the cooling device, which is to be used
> > + for the zone thermal control.
> > + If absent, cooling device controlled internally by
> > + the ASIC may be used.
> > +
> > +- trips : the nodes to describe a point in the
> temperature
> > + domain with key temperatures at which cooling is
> > + recommended. Each node must contain the next
> values:
> > + - type: the trip type. Expected values are:
> > + 0 - a trip point to enable active cooling;
> > + 1 - a trip point to enable passive cooling;
> > + 2 - a trip point to notify emergency;
> > + - temperature: unsigned integer indicating the trip
> > + temperature level in millicelsius;
> > + - minimum cooling state allowed within the trip
> node;
> > + - maximum cooling state allowed within the trip
> node;
> > +
> > +Example:
> > + asic_thermal: mlxsw_minimal@48 {
> > + compatible = "mlxsw_minimal";
>
> You missed the vendor part.
Acked.
>
> > + reg = <0x48>;
> > + status = "disabled";
>
> An example with it disabled?
We just use it in such way at BMC side. It's disabled by default and upon
event indicating the good health for the device the device driver is
connected. I can remove it from the example. But for BMC it's actually
the default state.
>
> > + cooling-phandle = <&cooling>;
> > +
> > + trips {
> > + trip@0 {
> > + trip = <0 75000 0 0>;
> > + };
>
> I don't know much about the thermal subsystem. But looking at other
> example binding documents, you seem to do something different here to
> other drivers. Why do you not use what seems to be the common format:
In mlxsw_thermal driver we have definition for the thermal trips, which contains
the type, like "active" of "passive", temperature in millicelsius and min/max states
for cooling device. These vector defines thermal trip points.
The hysteresis parameter is not relevant.
For example, ASIC thermal sensor is associated with the cooling device like:
&pwm_tacho {
...
cooling: fan@0 {
reg = <0x00>;
cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
aspeed,fan-tach-ch = /bits/ 8 <0x00>;
};
And the below sub-nodes
trip@0 {
trip = <0 75000 0 0>;
};
trip@1 {
trip = <2 85000 1 5>;
};
trip@3 {
trip = <2 105000 5 5>;
};
defines that PWM should be at default speed (125), while temperature is
below 75000, should be at max speed (255), while temperature is above
10500, and should step according the temperate trend between.
Thanks,
Vadim.
>
> trips {
> cpu_alert0: cpu-alert0 {
> temperature = <90000>; /* millicelsius */
> hysteresis = <2000>; /* millicelsius */
> type = "active";
> };
> cpu_alert1: cpu-alert1 {
> temperature = <100000>; /* millicelsius */
> hysteresis = <2000>; /* millicelsius */
> type = "passive";
> };
> cpu_crit: cpu-crit {
> temperature = <125000>; /* millicelsius */
> hysteresis = <2000>; /* millicelsius */
> type = "critical";
> };
> };
>
> Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net] sch_htb: fix crash on init failure
From: Nikolay Aleksandrov @ 2017-08-29 17:58 UTC (permalink / raw)
To: netdev; +Cc: edumazet, jhs, xiyou.wangcong, jiri, roopa, Nikolay Aleksandrov
The commit below added a call to the ->destroy() callback for all qdiscs
which failed in their ->init(), but some were not prepared for such
change and can't handle partially initialized qdisc. HTB is one of them
and if any error occurs before the qdisc watchdog timer and qdisc work are
initialized then we can hit either a null ptr deref (timer->base) when
canceling in ->destroy or lockdep error info about trying to register
a non-static key and a stack dump. So to fix these two move the watchdog
timer and workqueue init before anything that can err out.
To reproduce userspace needs to send broken htb qdisc create request,
tested with a modified tc (q_htb.c).
Trace log:
[ 2710.897602] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 2710.897977] IP: hrtimer_active+0x17/0x8a
[ 2710.898174] PGD 58fab067
[ 2710.898175] P4D 58fab067
[ 2710.898353] PUD 586c0067
[ 2710.898531] PMD 0
[ 2710.898710]
[ 2710.899045] Oops: 0000 [#1] SMP
[ 2710.899232] Modules linked in:
[ 2710.899419] CPU: 1 PID: 950 Comm: tc Not tainted 4.13.0-rc6+ #54
[ 2710.899646] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
[ 2710.900035] task: ffff880059ed2700 task.stack: ffff88005ad4c000
[ 2710.900262] RIP: 0010:hrtimer_active+0x17/0x8a
[ 2710.900467] RSP: 0018:ffff88005ad4f960 EFLAGS: 00010246
[ 2710.900684] RAX: 0000000000000000 RBX: ffff88003701e298 RCX: 0000000000000000
[ 2710.900933] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88003701e298
[ 2710.901177] RBP: ffff88005ad4f980 R08: 0000000000000001 R09: 0000000000000001
[ 2710.901419] R10: ffff88005ad4f800 R11: 0000000000000400 R12: 0000000000000000
[ 2710.901663] R13: ffff88003701e298 R14: ffffffff822a4540 R15: ffff88005ad4fac0
[ 2710.901907] FS: 00007f2f5e90f740(0000) GS:ffff88005d880000(0000) knlGS:0000000000000000
[ 2710.902277] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2710.902500] CR2: 0000000000000000 CR3: 0000000058ca3000 CR4: 00000000000406e0
[ 2710.902744] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 2710.902977] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 2710.903180] Call Trace:
[ 2710.903332] hrtimer_try_to_cancel+0x1a/0x93
[ 2710.903504] hrtimer_cancel+0x15/0x20
[ 2710.903667] qdisc_watchdog_cancel+0x12/0x14
[ 2710.903866] htb_destroy+0x2e/0xf7
[ 2710.904097] qdisc_create+0x377/0x3fd
[ 2710.904330] tc_modify_qdisc+0x4d2/0x4fd
[ 2710.904511] rtnetlink_rcv_msg+0x188/0x197
[ 2710.904682] ? rcu_read_unlock+0x3e/0x5f
[ 2710.904849] ? rtnl_newlink+0x729/0x729
[ 2710.905017] netlink_rcv_skb+0x6c/0xce
[ 2710.905183] rtnetlink_rcv+0x23/0x2a
[ 2710.905345] netlink_unicast+0x103/0x181
[ 2710.905511] netlink_sendmsg+0x326/0x337
[ 2710.905679] sock_sendmsg_nosec+0x14/0x3f
[ 2710.905847] sock_sendmsg+0x29/0x2e
[ 2710.906010] ___sys_sendmsg+0x209/0x28b
[ 2710.906176] ? do_raw_spin_unlock+0xcd/0xf8
[ 2710.906346] ? _raw_spin_unlock+0x27/0x31
[ 2710.906514] ? __handle_mm_fault+0x651/0xdb1
[ 2710.906685] ? check_chain_key+0xb0/0xfd
[ 2710.906855] __sys_sendmsg+0x45/0x63
[ 2710.907018] ? __sys_sendmsg+0x45/0x63
[ 2710.907185] SyS_sendmsg+0x19/0x1b
[ 2710.907344] entry_SYSCALL_64_fastpath+0x23/0xc2
Note that probably this bug goes further back because the default qdisc
handling always calls ->destroy on init failure too.
Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
Always calling qdisc destroy on init failure in the default qdisc handling
was added in commit 0fbbeb1ba43b. I'm not sure if I should include that
one as fixes tag.
There're more fixes to come, some are much easier to trigger without
modifications to tc.
net/sched/sch_htb.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 5d65ec5207e9..5bf5177b2bd3 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1017,6 +1017,9 @@ static int htb_init(struct Qdisc *sch, struct nlattr *opt)
int err;
int i;
+ qdisc_watchdog_init(&q->watchdog, sch);
+ INIT_WORK(&q->work, htb_work_func);
+
if (!opt)
return -EINVAL;
@@ -1041,8 +1044,6 @@ static int htb_init(struct Qdisc *sch, struct nlattr *opt)
for (i = 0; i < TC_HTB_NUMPRIO; i++)
INIT_LIST_HEAD(q->drops + i);
- qdisc_watchdog_init(&q->watchdog, sch);
- INIT_WORK(&q->work, htb_work_func);
qdisc_skb_head_init(&q->direct_queue);
if (tb[TCA_HTB_DIRECT_QLEN])
--
2.1.4
^ permalink raw reply related
* Re: UDP sockets oddities
From: Eric Dumazet @ 2017-08-29 18:01 UTC (permalink / raw)
To: Florian Fainelli; +Cc: David Miller, netdev, pabeni, willemb
In-Reply-To: <353aa37f-c62b-f5af-8c89-f67af6509497@gmail.com>
On Tue, 2017-08-29 at 10:53 -0700, Florian Fainelli wrote:
> On 08/26/2017 11:56 AM, Florian Fainelli wrote:
> >
> >
> > On 08/26/2017 05:47 AM, Eric Dumazet wrote:
> >> On Fri, 2017-08-25 at 21:19 -0700, David Miller wrote:
> >>
> >>> Agreed, but the ARP resolution queue really needs to scale it's backlog
> >>> to the physical technology it is attached to.
> >> Yes, last time (in 2011) we increased the old limit of 3 packets :/
> >>
> >> We probably should match sysctl_wmem_max so that a single socket
> >> provider would hit its sk_sndbuf limit
>
> Eric, do you want to post this as a formal patch? I don't think I
> understand these tunables enough to provide a good commit message
> anyways. Thanks!
I will post it today.
I was out of the office yesterday, rafting on the south fork of American
River ;)
This will target net-next.
Thanks.
^ permalink raw reply
* Re: pull-request: wireless-drivers-next 2017-08-28
From: David Miller @ 2017-08-29 18:05 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87val8ujvp.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Mon, 28 Aug 2017 12:22:34 +0300
> here's a pull request to net-next for 4.14. Because I pulled
> wireless-drivers (at least that's my suspicion) the diffstat was wrong
> again and I created it manually. I recall Linus somewhere saying that in
> certain cases this is normal and it's ok to create the diffstat
> manually, so I don't worry about this anymore.
Yeah, that's fine.
> In this pull request we also add SDIO_DEVICE_ID_CYPRESS_4373 to
> include/linux/mmc/sdio_ids.h which stands out in the diffstat.
>
> Please let me know if there are any problems.
Pulled, thanks!
^ permalink raw reply
* Re: [PATCH net] sch_htb: fix crash on init failure
From: Eric Dumazet @ 2017-08-29 18:09 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, edumazet, jhs, xiyou.wangcong, jiri, roopa
In-Reply-To: <1504029487-7085-1-git-send-email-nikolay@cumulusnetworks.com>
On Tue, 2017-08-29 at 20:58 +0300, Nikolay Aleksandrov wrote:
> The commit below added a call to the ->destroy() callback for all qdiscs
> which failed in their ->init(), but some were not prepared for such
> change and can't handle partially initialized qdisc. HTB is one of them
> and if any error occurs before the qdisc watchdog timer and qdisc work are
> initialized then we can hit either a null ptr deref (timer->base) when
> canceling in ->destroy or lockdep error info about trying to register
> a non-static key and a stack dump. So to fix these two move the watchdog
> timer and workqueue init before anything that can err out.
> To reproduce userspace needs to send broken htb qdisc create request,
> tested with a modified tc (q_htb.c).
> Note that probably this bug goes further back because the default qdisc
> handling always calls ->destroy on init failure too.
>
> Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> Always calling qdisc destroy on init failure in the default qdisc handling
> was added in commit 0fbbeb1ba43b. I'm not sure if I should include that
> one as fixes tag.
Well, we probably need to audit init/destroy not only in net/sched, but
other parts of networking stack.
What about the qdisc_skb_head_init(&q->direct_queue) call ?
I am surprised you do not crash in __skb_queue_purge(&q->direct_queue);
^ permalink raw reply
* Re: [PATCH net] sch_htb: fix crash on init failure
From: Cong Wang @ 2017-08-29 18:13 UTC (permalink / raw)
To: Eric Dumazet
Cc: Nikolay Aleksandrov, Linux Kernel Network Developers,
Eric Dumazet, Jamal Hadi Salim, Jiri Pirko, Roopa Prabhu
In-Reply-To: <1504030196.11498.82.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, Aug 29, 2017 at 11:09 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2017-08-29 at 20:58 +0300, Nikolay Aleksandrov wrote:
>> The commit below added a call to the ->destroy() callback for all qdiscs
>> which failed in their ->init(), but some were not prepared for such
>> change and can't handle partially initialized qdisc. HTB is one of them
>> and if any error occurs before the qdisc watchdog timer and qdisc work are
>> initialized then we can hit either a null ptr deref (timer->base) when
>> canceling in ->destroy or lockdep error info about trying to register
>> a non-static key and a stack dump. So to fix these two move the watchdog
>> timer and workqueue init before anything that can err out.
>> To reproduce userspace needs to send broken htb qdisc create request,
>> tested with a modified tc (q_htb.c).
>
>> Note that probably this bug goes further back because the default qdisc
>> handling always calls ->destroy on init failure too.
>>
>> Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation")
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>> Always calling qdisc destroy on init failure in the default qdisc handling
>> was added in commit 0fbbeb1ba43b. I'm not sure if I should include that
>> one as fixes tag.
>
> Well, we probably need to audit init/destroy not only in net/sched, but
> other parts of networking stack.
>
> What about the qdisc_skb_head_init(&q->direct_queue) call ?
It just zero the pointers:
static inline void qdisc_skb_head_init(struct qdisc_skb_head *qh)
{
qh->head = NULL;
qh->tail = NULL;
qh->qlen = 0;
}
And qdisc is already kzalloc()'ed.
^ permalink raw reply
* Re: [PATCH net] sch_htb: fix crash on init failure
From: Eric Dumazet @ 2017-08-29 18:14 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, edumazet, jhs, xiyou.wangcong, jiri, roopa
In-Reply-To: <1504030196.11498.82.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, 2017-08-29 at 11:09 -0700, Eric Dumazet wrote:
> On Tue, 2017-08-29 at 20:58 +0300, Nikolay Aleksandrov wrote:
> > The commit below added a call to the ->destroy() callback for all qdiscs
> > which failed in their ->init(), but some were not prepared for such
> > change and can't handle partially initialized qdisc. HTB is one of them
> > and if any error occurs before the qdisc watchdog timer and qdisc work are
> > initialized then we can hit either a null ptr deref (timer->base) when
> > canceling in ->destroy or lockdep error info about trying to register
> > a non-static key and a stack dump. So to fix these two move the watchdog
> > timer and workqueue init before anything that can err out.
> > To reproduce userspace needs to send broken htb qdisc create request,
> > tested with a modified tc (q_htb.c).
>
> > Note that probably this bug goes further back because the default qdisc
> > handling always calls ->destroy on init failure too.
> >
> > Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation")
> > Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> > ---
> > Always calling qdisc destroy on init failure in the default qdisc handling
> > was added in commit 0fbbeb1ba43b. I'm not sure if I should include that
> > one as fixes tag.
>
> Well, we probably need to audit init/destroy not only in net/sched, but
> other parts of networking stack.
>
> What about the qdisc_skb_head_init(&q->direct_queue) call ?
>
> I am surprised you do not crash in __skb_queue_purge(&q->direct_queue);
Oh, this is because skb_peek() is happy if queue->next is NULL.
^ permalink raw reply
* Re: [PATCH net] sch_htb: fix crash on init failure
From: Nikolay Aleksandrov @ 2017-08-29 18:19 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, edumazet, jhs, xiyou.wangcong, jiri, roopa
In-Reply-To: <1504030196.11498.82.camel@edumazet-glaptop3.roam.corp.google.com>
On 29/08/17 21:09, Eric Dumazet wrote:
> On Tue, 2017-08-29 at 20:58 +0300, Nikolay Aleksandrov wrote:
>> The commit below added a call to the ->destroy() callback for all qdiscs
>> which failed in their ->init(), but some were not prepared for such
>> change and can't handle partially initialized qdisc. HTB is one of them
>> and if any error occurs before the qdisc watchdog timer and qdisc work are
>> initialized then we can hit either a null ptr deref (timer->base) when
>> canceling in ->destroy or lockdep error info about trying to register
>> a non-static key and a stack dump. So to fix these two move the watchdog
>> timer and workqueue init before anything that can err out.
>> To reproduce userspace needs to send broken htb qdisc create request,
>> tested with a modified tc (q_htb.c).
>
>> Note that probably this bug goes further back because the default qdisc
>> handling always calls ->destroy on init failure too.
>>
>> Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation")
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>> Always calling qdisc destroy on init failure in the default qdisc handling
>> was added in commit 0fbbeb1ba43b. I'm not sure if I should include that
>> one as fixes tag.
>
> Well, we probably need to audit init/destroy not only in net/sched, but
> other parts of networking stack.
>
I'm not sure I follow, I hit this while working on a net/sched/ patch and had to error
out in the init() function.
> What about the qdisc_skb_head_init(&q->direct_queue) call ?
>
> I am surprised you do not crash in __skb_queue_purge(&q->direct_queue);
>
Hm, do you mean in __qdisc_reset_queue() ?
I have only tried/seen the crash happen on qdisc add.
A much simpler and easier bug is sch_multiq (ethX non-multiq):
$ tc qdisc add dev ethX root multiq
(error EOPNOTSUPP + double free of ->queues due to free in init())
and e.g. $ ip l add dumdum type dummy
to see a crash due to the corrupted memory.
^ permalink raw reply
* Re: [PATCH net] sch_htb: fix crash on init failure
From: Cong Wang @ 2017-08-29 18:20 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Linux Kernel Network Developers, Eric Dumazet, Jamal Hadi Salim,
Jiri Pirko, Roopa Prabhu
In-Reply-To: <1504029487-7085-1-git-send-email-nikolay@cumulusnetworks.com>
On Tue, Aug 29, 2017 at 10:58 AM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> The commit below added a call to the ->destroy() callback for all qdiscs
> which failed in their ->init(), but some were not prepared for such
> change and can't handle partially initialized qdisc. HTB is one of them
> and if any error occurs before the qdisc watchdog timer and qdisc work are
> initialized then we can hit either a null ptr deref (timer->base) when
> canceling in ->destroy or lockdep error info about trying to register
> a non-static key and a stack dump. So to fix these two move the watchdog
> timer and workqueue init before anything that can err out.
> To reproduce userspace needs to send broken htb qdisc create request,
> tested with a modified tc (q_htb.c).
>
> Trace log:
> [ 2710.897602] BUG: unable to handle kernel NULL pointer dereference at (null)
> [ 2710.897977] IP: hrtimer_active+0x17/0x8a
> [ 2710.898174] PGD 58fab067
> [ 2710.898175] P4D 58fab067
> [ 2710.898353] PUD 586c0067
> [ 2710.898531] PMD 0
> [ 2710.898710]
> [ 2710.899045] Oops: 0000 [#1] SMP
> [ 2710.899232] Modules linked in:
> [ 2710.899419] CPU: 1 PID: 950 Comm: tc Not tainted 4.13.0-rc6+ #54
> [ 2710.899646] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
> [ 2710.900035] task: ffff880059ed2700 task.stack: ffff88005ad4c000
> [ 2710.900262] RIP: 0010:hrtimer_active+0x17/0x8a
> [ 2710.900467] RSP: 0018:ffff88005ad4f960 EFLAGS: 00010246
> [ 2710.900684] RAX: 0000000000000000 RBX: ffff88003701e298 RCX: 0000000000000000
> [ 2710.900933] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88003701e298
> [ 2710.901177] RBP: ffff88005ad4f980 R08: 0000000000000001 R09: 0000000000000001
> [ 2710.901419] R10: ffff88005ad4f800 R11: 0000000000000400 R12: 0000000000000000
> [ 2710.901663] R13: ffff88003701e298 R14: ffffffff822a4540 R15: ffff88005ad4fac0
> [ 2710.901907] FS: 00007f2f5e90f740(0000) GS:ffff88005d880000(0000) knlGS:0000000000000000
> [ 2710.902277] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 2710.902500] CR2: 0000000000000000 CR3: 0000000058ca3000 CR4: 00000000000406e0
> [ 2710.902744] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 2710.902977] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 2710.903180] Call Trace:
> [ 2710.903332] hrtimer_try_to_cancel+0x1a/0x93
> [ 2710.903504] hrtimer_cancel+0x15/0x20
> [ 2710.903667] qdisc_watchdog_cancel+0x12/0x14
> [ 2710.903866] htb_destroy+0x2e/0xf7
> [ 2710.904097] qdisc_create+0x377/0x3fd
> [ 2710.904330] tc_modify_qdisc+0x4d2/0x4fd
> [ 2710.904511] rtnetlink_rcv_msg+0x188/0x197
> [ 2710.904682] ? rcu_read_unlock+0x3e/0x5f
> [ 2710.904849] ? rtnl_newlink+0x729/0x729
> [ 2710.905017] netlink_rcv_skb+0x6c/0xce
> [ 2710.905183] rtnetlink_rcv+0x23/0x2a
> [ 2710.905345] netlink_unicast+0x103/0x181
> [ 2710.905511] netlink_sendmsg+0x326/0x337
> [ 2710.905679] sock_sendmsg_nosec+0x14/0x3f
> [ 2710.905847] sock_sendmsg+0x29/0x2e
> [ 2710.906010] ___sys_sendmsg+0x209/0x28b
> [ 2710.906176] ? do_raw_spin_unlock+0xcd/0xf8
> [ 2710.906346] ? _raw_spin_unlock+0x27/0x31
> [ 2710.906514] ? __handle_mm_fault+0x651/0xdb1
> [ 2710.906685] ? check_chain_key+0xb0/0xfd
> [ 2710.906855] __sys_sendmsg+0x45/0x63
> [ 2710.907018] ? __sys_sendmsg+0x45/0x63
> [ 2710.907185] SyS_sendmsg+0x19/0x1b
> [ 2710.907344] entry_SYSCALL_64_fastpath+0x23/0xc2
>
> Note that probably this bug goes further back because the default qdisc
> handling always calls ->destroy on init failure too.
>
> Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> Always calling qdisc destroy on init failure in the default qdisc handling
> was added in commit 0fbbeb1ba43b. I'm not sure if I should include that
> one as fixes tag.
>
> There're more fixes to come, some are much easier to trigger without
> modifications to tc.
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply
* Re: [PATCH net] sch_htb: fix crash on init failure
From: Eric Dumazet @ 2017-08-29 18:23 UTC (permalink / raw)
To: Cong Wang
Cc: Nikolay Aleksandrov, Linux Kernel Network Developers,
Eric Dumazet, Jamal Hadi Salim, Jiri Pirko, Roopa Prabhu
In-Reply-To: <CAM_iQpUh2LjwuzNQ1OqRRzKMJ1uARp0ouOQ42HEsAC5XJm_BWg@mail.gmail.com>
On Tue, 2017-08-29 at 11:13 -0700, Cong Wang wrote:
> On Tue, Aug 29, 2017 at 11:09 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Tue, 2017-08-29 at 20:58 +0300, Nikolay Aleksandrov wrote:
> >> The commit below added a call to the ->destroy() callback for all qdiscs
> >> which failed in their ->init(), but some were not prepared for such
> >> change and can't handle partially initialized qdisc. HTB is one of them
> >> and if any error occurs before the qdisc watchdog timer and qdisc work are
> >> initialized then we can hit either a null ptr deref (timer->base) when
> >> canceling in ->destroy or lockdep error info about trying to register
> >> a non-static key and a stack dump. So to fix these two move the watchdog
> >> timer and workqueue init before anything that can err out.
> >> To reproduce userspace needs to send broken htb qdisc create request,
> >> tested with a modified tc (q_htb.c).
> >
> >> Note that probably this bug goes further back because the default qdisc
> >> handling always calls ->destroy on init failure too.
> >>
> >> Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation")
> >> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> >> ---
> >> Always calling qdisc destroy on init failure in the default qdisc handling
> >> was added in commit 0fbbeb1ba43b. I'm not sure if I should include that
> >> one as fixes tag.
> >
> > Well, we probably need to audit init/destroy not only in net/sched, but
> > other parts of networking stack.
> >
> > What about the qdisc_skb_head_init(&q->direct_queue) call ?
>
> It just zero the pointers:
>
> static inline void qdisc_skb_head_init(struct qdisc_skb_head *qh)
> {
> qh->head = NULL;
> qh->tail = NULL;
> qh->qlen = 0;
> }
>
> And qdisc is already kzalloc()'ed.
Yeah, I was looking at some old tree, not having the
__qdisc_reset_queue() yet.
^ permalink raw reply
* [PATCH net] sch_multiq: fix double free on init failure
From: Nikolay Aleksandrov @ 2017-08-29 18:26 UTC (permalink / raw)
To: netdev; +Cc: edumazet, jhs, xiyou.wangcong, jiri, roopa, Nikolay Aleksandrov
The below commit added a call to ->destroy() on init failure, but multiq
still frees ->queues on error in init, but ->queues is also freed by
->destroy() thus we get double free and corrupted memory.
Very easy to reproduce (eth0 not multiqueue):
$ tc qdisc add dev eth0 root multiq
RTNETLINK answers: Operation not supported
$ ip l add dumdum type dummy
(crash)
Trace log:
[ 3929.467747] general protection fault: 0000 [#1] SMP
[ 3929.468083] Modules linked in:
[ 3929.468302] CPU: 3 PID: 967 Comm: ip Not tainted 4.13.0-rc6+ #56
[ 3929.468625] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
[ 3929.469124] task: ffff88003716a700 task.stack: ffff88005872c000
[ 3929.469449] RIP: 0010:__kmalloc_track_caller+0x117/0x1be
[ 3929.469746] RSP: 0018:ffff88005872f6a0 EFLAGS: 00010246
[ 3929.470042] RAX: 00000000000002de RBX: 0000000058a59000 RCX: 00000000000002df
[ 3929.470406] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff821f7020
[ 3929.470770] RBP: ffff88005872f6e8 R08: 000000000001f010 R09: 0000000000000000
[ 3929.471133] R10: ffff88005872f730 R11: 0000000000008cdd R12: ff006d75646d7564
[ 3929.471496] R13: 00000000014000c0 R14: ffff88005b403c00 R15: ffff88005b403c00
[ 3929.471869] FS: 00007f0b70480740(0000) GS:ffff88005d980000(0000) knlGS:0000000000000000
[ 3929.472286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3929.472677] CR2: 00007ffcee4f3000 CR3: 0000000059d45000 CR4: 00000000000406e0
[ 3929.473209] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 3929.474109] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 3929.474873] Call Trace:
[ 3929.475337] ? kstrdup_const+0x23/0x25
[ 3929.475863] kstrdup+0x2e/0x4b
[ 3929.476338] kstrdup_const+0x23/0x25
[ 3929.478084] __kernfs_new_node+0x28/0xbc
[ 3929.478478] kernfs_new_node+0x35/0x55
[ 3929.478929] kernfs_create_link+0x23/0x76
[ 3929.479478] sysfs_do_create_link_sd.isra.2+0x85/0xd7
[ 3929.480096] sysfs_create_link+0x33/0x35
[ 3929.480649] device_add+0x200/0x589
[ 3929.481184] netdev_register_kobject+0x7c/0x12f
[ 3929.481711] register_netdevice+0x373/0x471
[ 3929.482174] rtnl_newlink+0x614/0x729
[ 3929.482610] ? rtnl_newlink+0x17f/0x729
[ 3929.483080] rtnetlink_rcv_msg+0x188/0x197
[ 3929.483533] ? rcu_read_unlock+0x3e/0x5f
[ 3929.483984] ? rtnl_newlink+0x729/0x729
[ 3929.484420] netlink_rcv_skb+0x6c/0xce
[ 3929.484858] rtnetlink_rcv+0x23/0x2a
[ 3929.485291] netlink_unicast+0x103/0x181
[ 3929.485735] netlink_sendmsg+0x326/0x337
[ 3929.486181] sock_sendmsg_nosec+0x14/0x3f
[ 3929.486614] sock_sendmsg+0x29/0x2e
[ 3929.486973] ___sys_sendmsg+0x209/0x28b
[ 3929.487340] ? do_raw_spin_unlock+0xcd/0xf8
[ 3929.487719] ? _raw_spin_unlock+0x27/0x31
[ 3929.488092] ? __handle_mm_fault+0x651/0xdb1
[ 3929.488471] ? check_chain_key+0xb0/0xfd
[ 3929.488847] __sys_sendmsg+0x45/0x63
[ 3929.489206] ? __sys_sendmsg+0x45/0x63
[ 3929.489576] SyS_sendmsg+0x19/0x1b
[ 3929.489901] entry_SYSCALL_64_fastpath+0x23/0xc2
[ 3929.490172] RIP: 0033:0x7f0b6fb93690
[ 3929.490423] RSP: 002b:00007ffcee4ed588 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[ 3929.490881] RAX: ffffffffffffffda RBX: ffffffff810d278c RCX: 00007f0b6fb93690
[ 3929.491198] RDX: 0000000000000000 RSI: 00007ffcee4ed5d0 RDI: 0000000000000003
[ 3929.491521] RBP: ffff88005872ff98 R08: 0000000000000001 R09: 0000000000000000
[ 3929.491801] R10: 00007ffcee4ed350 R11: 0000000000000246 R12: 0000000000000002
[ 3929.492075] R13: 000000000066f1a0 R14: 00007ffcee4f5680 R15: 0000000000000000
[ 3929.492352] ? trace_hardirqs_off_caller+0xa7/0xcf
[ 3929.492590] Code: 8b 45 c0 48 8b 45 b8 74 17 48 8b 4d c8 83 ca ff 44
89 ee 4c 89 f7 e8 83 ca ff ff 49 89 c4 eb 49 49 63 56 20 48 8d 48 01 4d
8b 06 <49> 8b 1c 14 48 89 c2 4c 89 e0 65 49 0f c7 08 0f 94 c0 83 f0 01
[ 3929.493335] RIP: __kmalloc_track_caller+0x117/0x1be RSP: ffff88005872f6a0
Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/sched/sch_multiq.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
index f143b7bbaa0d..b07f8b01aa07 100644
--- a/net/sched/sch_multiq.c
+++ b/net/sched/sch_multiq.c
@@ -259,9 +259,6 @@ static int multiq_init(struct Qdisc *sch, struct nlattr *opt)
err = multiq_tune(sch, opt);
- if (err)
- kfree(q->queues);
-
return err;
}
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net] sch_multiq: fix double free on init failure
From: Eric Dumazet @ 2017-08-29 18:40 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, edumazet, jhs, xiyou.wangcong, jiri, roopa
In-Reply-To: <1504031188-11434-1-git-send-email-nikolay@cumulusnetworks.com>
On Tue, 2017-08-29 at 21:26 +0300, Nikolay Aleksandrov wrote:
> The below commit added a call to ->destroy() on init failure, but multiq
> still frees ->queues on error in init, but ->queues is also freed by
> ->destroy() thus we get double free and corrupted memory.
>
> Very easy to reproduce (eth0 not multiqueue):
> $ tc qdisc add dev eth0 root multiq
> RTNETLINK answers: Operation not supported
> $ ip l add dumdum type dummy
> (crash)
> Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> net/sched/sch_multiq.c | 3 ---
> 1 file changed, 3 deletions(-)
>
Acked-by: Eric Dumazet <edumazet@google.com>
^ 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