Netdev List
 help / color / mirror / Atom feed
* [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: nixiaoming @ 2017-09-14  2:44 UTC (permalink / raw)
  To: davem, edumazet, waltje, gw4pts, andreyknvl, tklauser,
	philip.pettersson, glider
  Cc: netdev, linux-kernel, nixiaoming, dede.wu

If fanout_add is preempted after running po-> fanout = match
and before running __fanout_link,
it will cause BUG_ON when __unregister_prot_hook call __fanout_unlink

so, we need add mutex_lock(&fanout_mutex) to __unregister_prot_hook
or add spin_lock(&po->bind_lock) before po-> fanout = match

test on linux 4.1.12:
./trinity -c setsockopt -C 2 -X &

BUG: failure at net/packet/af_packet.c:1414/__fanout_unlink()!
Kernel panic - not syncing: BUG!
CPU: 2 PID: 2271 Comm: trinity-c0 Tainted: G        W  O    4.1.12 #1
Hardware name: Hisilicon PhosphorHi1382 FPGA (DT)
Call trace:
[<ffffffc000209414>] dump_backtrace+0x0/0xf8
[<ffffffc00020952c>] show_stack+0x20/0x28
[<ffffffc000635574>] dump_stack+0xac/0xe4
[<ffffffc000633fb8>] panic+0xf8/0x268
[<ffffffc0005fa778>] __unregister_prot_hook+0xa0/0x144
[<ffffffc0005fba48>] packet_set_ring+0x280/0x5b4
[<ffffffc0005fc33c>] packet_setsockopt+0x320/0x950
[<ffffffc000554a04>] SyS_setsockopt+0xa4/0xd4

Signed-off-by: nixiaoming <nixiaoming@huawei.com>
Tested-by: wudesheng <dede.wu@huawei.com>
---
 net/packet/af_packet.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 008a45c..0300146 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -365,10 +365,12 @@ static void __unregister_prot_hook(struct sock *sk, bool sync)
 
 	po->running = 0;
 
+	mutex_lock(&fanout_mutex);
 	if (po->fanout)
 		__fanout_unlink(sk, po);
 	else
 		__dev_remove_pack(&po->prot_hook);
+	mutex_unlock(&fanout_mutex);
 
 	__sock_put(sk);
 
-- 
2.11.0.1

^ permalink raw reply related

* [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: nixiaoming @ 2017-09-14  2:40 UTC (permalink / raw)
  To: davem, edumazet, waltje, gw4pts, andreyknvl, tklauser,
	philip.pettersson, glider
  Cc: netdev, linux-kernel, nixiaoming, dede.wu

If fanout_add is preempted after running po-> fanout = match
and before running __fanout_link,
it will cause BUG_ON when __unregister_prot_hook call __fanout_unlink

so, we need add mutex_lock(&fanout_mutex) to __unregister_prot_hook
or add spin_lock(&po->bind_lock) before po-> fanout = match

test on linux 4.1.42:
./trinity -c setsockopt -C 2 -X &

BUG: failure at net/packet/af_packet.c:1414/__fanout_unlink()!
Kernel panic - not syncing: BUG!
CPU: 2 PID: 2271 Comm: trinity-c0 Tainted: G        W  O    4.1.12 #1
Hardware name: Hisilicon PhosphorHi1382 FPGA (DT)
Call trace:
[<ffffffc000209414>] dump_backtrace+0x0/0xf8
[<ffffffc00020952c>] show_stack+0x20/0x28
[<ffffffc000635574>] dump_stack+0xac/0xe4
[<ffffffc000633fb8>] panic+0xf8/0x268
[<ffffffc0005fa778>] __unregister_prot_hook+0xa0/0x144
[<ffffffc0005fba48>] packet_set_ring+0x280/0x5b4
[<ffffffc0005fc33c>] packet_setsockopt+0x320/0x950
[<ffffffc000554a04>] SyS_setsockopt+0xa4/0xd4

Signed-off-by: nixiaoming <nixiaoming@huawei.com>
Tested-by: wudesheng <dede.wu@huawei.com>
---
 net/packet/af_packet.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 008a45c..0300146 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -365,10 +365,12 @@ static void __unregister_prot_hook(struct sock *sk, bool sync)
 
 	po->running = 0;
 
+	mutex_lock(&fanout_mutex);
 	if (po->fanout)
 		__fanout_unlink(sk, po);
 	else
 		__dev_remove_pack(&po->prot_hook);
+	mutex_unlock(&fanout_mutex);
 
 	__sock_put(sk);
 
-- 
2.11.0.1

^ permalink raw reply related

* RE: [Intel-wired-lan] [PATCH] igb: check memory allocation failure
From: Brown, Aaron F @ 2017-09-14  2:24 UTC (permalink / raw)
  To: Christophe JAILLET, Waskiewicz Jr, Peter, Kirsher, Jeffrey T
  Cc: netdev@vger.kernel.org, kernel-janitors@vger.kernel.org,
	intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org
In-Reply-To: <95a0bc80-8b41-d74f-8059-ae7181493120@wanadoo.fr>

> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of Christophe JAILLET
> Sent: Monday, August 28, 2017 10:13 AM
> To: Waskiewicz Jr, Peter <peter.waskiewicz.jr@intel.com>; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Cc: netdev@vger.kernel.org; kernel-janitors@vger.kernel.org; intel-wired-
> lan@lists.osuosl.org; linux-kernel@vger.kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH] igb: check memory allocation failure
> 
> Le 28/08/2017 à 01:09, Waskiewicz Jr, Peter a écrit :
> > On 8/27/17 2:42 AM, Christophe JAILLET wrote:
> >> Check memory allocation failures and return -ENOMEM in such cases, as
> >> already done for other memory allocations in this function.
> >>
> >> This avoids NULL pointers dereference.
> >>
> >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> >> ---
> >>    drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
> >>    1 file changed, 2 insertions(+)
> >>

This seems to be fine from a "it does not break in testing" perspective, so...

Tested-by: Aaron Brown <aaron.f.brown@intel.com

> > -PJ
> >
> Hi,
> 
> in fact, there is no leak because the only caller of 'igb_sw_init()'
> (i.e. 'igb_probe()'), already frees these resources in case of error,
> see [1]
> 
> These resources are also freed  in 'igb_remove()'.
> 
> Best reagrds,
> CJ
> 
> [1]:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
> next.git/tree/drivers/net/ethernet/intel/igb/igb_main.c#n2775

But is PJ's comment saying that it is not really necessary?  If so I tend to lean towards the don't touch it if it's not broken perspective.
 


^ permalink raw reply

* [PATCH net] udpv6: Fix the checksum computation when HW checksum does not apply
From: Subash Abhinov Kasiviswanathan @ 2017-09-14  1:30 UTC (permalink / raw)
  To: netdev, vyasevic; +Cc: Subash Abhinov Kasiviswanathan

While trying an ESP transport mode encryption for UDPv6 packets of
datagram size 1436 with MTU 1500, checksum error was observed in
the secondary fragment.

This error occurs due to the UDP payload checksum being missed out
when computing the full checksum for these packets in
udp6_hwcsum_outgoing().

Fixes: d39d938c8228 ("ipv6: Introduce udpv6_send_skb()")
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 net/ipv6/udp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index e2ecfb1..40d7234 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1015,6 +1015,7 @@ static void udp6_hwcsum_outgoing(struct sock *sk, struct sk_buff *skb,
 		 */
 		offset = skb_transport_offset(skb);
 		skb->csum = skb_checksum(skb, offset, skb->len - offset, 0);
+		csum = skb->csum;
 
 		skb->ip_summed = CHECKSUM_NONE;
 
-- 
1.9.1

^ permalink raw reply related

* Re: [RFC net-next 2/5] net/sched: Introduce Credit Based Shaper (CBS) qdisc
From: Vinicius Costa Gomes @ 2017-09-14  0:39 UTC (permalink / raw)
  To: Henrik Austad
  Cc: netdev, jhs, xiyou.wangcong, jiri, intel-wired-lan, andre.guedes,
	ivan.briano, jesus.sanchez-palencia, boon.leong.ong,
	richardcochran
In-Reply-To: <20170908134332.GD9064@sisyphus.home.austad.us>

Hi Henrik,

Henrik Austad <henrik@austad.us> writes:

> On Thu, Aug 31, 2017 at 06:26:22PM -0700, Vinicius Costa Gomes wrote:
>> This queueing discipline implements the shaper algorithm defined by
>> the 802.1Q-2014 Section 8.6.8.2 and detailed in Annex L.
>>
>> It's primary usage is to apply some bandwidth reservation to user
>> defined traffic classes, which are mapped to different queues via the
>> mqprio qdisc.
>>
>> Initially, it only supports offloading the traffic shaping work to
>> supporting controllers.
>>
>> Later, when a software implementation is added, the current dependency
>> on being installed "under" mqprio can be lifted.
>>
>> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
>> Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
>
> So, I started testing this in my VM to make sure I didn't do anything silly
> and it exploded spectacularly as it used the underlying e1000 driver which
> does not have a ndo_setup_tc present.
>
> I commented inline below, but as a tl;dr
>
> The cbs_init() would call into cbs_change() that would correctly detect
> that ndo_setup_tc is missing and abort. However, at this stage it would try
> to desroy the qdisc and in cbs_destroy() there's no such guard leading to a
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
>
> Fixing that, another NULL would be found when the code walks into
> qdisc_destroy(q->qdisc).
>
> Apart from this, it loaded fine after some wrestling with tc :)
>
> Awesome! :D
>
>> ---
>>  include/linux/netdevice.h |   1 +
>>  net/sched/Kconfig         |  11 ++
>>  net/sched/Makefile        |   1 +
>>  net/sched/sch_cbs.c       | 286 ++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 299 insertions(+)
>>  create mode 100644 net/sched/sch_cbs.c
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 35de8312e0b5..dd9a2ecd0c03 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -775,6 +775,7 @@ enum tc_setup_type {
>>  	TC_SETUP_CLSFLOWER,
>>  	TC_SETUP_CLSMATCHALL,
>>  	TC_SETUP_CLSBPF,
>> +	TC_SETUP_CBS,
>>  };
>>
>>  /* These structures hold the attributes of xdp state that are being passed
>> diff --git a/net/sched/Kconfig b/net/sched/Kconfig
>> index e70ed26485a2..c03d86a7775e 100644
>> --- a/net/sched/Kconfig
>> +++ b/net/sched/Kconfig
>> @@ -172,6 +172,17 @@ config NET_SCH_TBF
>>  	  To compile this code as a module, choose M here: the
>>  	  module will be called sch_tbf.
>>
>> +config NET_SCH_CBS
>
> Shouldn't this depend on NET_SCH_MQPRIO as it is supposed to hook into
> that?

Right now, the CBS shaper only makes sense with mqprio, later it may use
some other qdisc (like "taprio" mentioned in the cover letter) to hook
into, so, yes, you are right, there's a dependency here, better make it
clear. Will fix.

>
> @@ -173,6 +173,7 @@ config NET_SCH_TBF
>           module will be called sch_tbf.
>
>  config NET_SCH_CBS
> +       depends on NET_SCH_MQPRIO
>         tristate "Credit Based Shaper (CBS)"
>         ---help---
>           Say Y here if you want to use the Credit Based Shaper (CBS) packet
>
>> +	tristate "Credit Based Shaper (CBS)"
>> +	---help---
>> +	  Say Y here if you want to use the Credit Based Shaper (CBS) packet
>> +	  scheduling algorithm.
>> +
>> +	  See the top of <file:net/sched/sch_cbs.c> for more details.
>> +
>> +	  To compile this code as a module, choose M here: the
>> +	  module will be called sch_cbs.
>> +
>>  config NET_SCH_GRED
>>  	tristate "Generic Random Early Detection (GRED)"
>>  	---help---
>> diff --git a/net/sched/Makefile b/net/sched/Makefile
>> index 7b915d226de7..80c8f92d162d 100644
>> --- a/net/sched/Makefile
>> +++ b/net/sched/Makefile
>> @@ -52,6 +52,7 @@ obj-$(CONFIG_NET_SCH_FQ_CODEL)	+= sch_fq_codel.o
>>  obj-$(CONFIG_NET_SCH_FQ)	+= sch_fq.o
>>  obj-$(CONFIG_NET_SCH_HHF)	+= sch_hhf.o
>>  obj-$(CONFIG_NET_SCH_PIE)	+= sch_pie.o
>> +obj-$(CONFIG_NET_SCH_CBS)	+= sch_cbs.o
>>
>>  obj-$(CONFIG_NET_CLS_U32)	+= cls_u32.o
>>  obj-$(CONFIG_NET_CLS_ROUTE4)	+= cls_route.o
>> diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
>> new file mode 100644
>> index 000000000000..1c86a9e14150
>> --- /dev/null
>> +++ b/net/sched/sch_cbs.c
>> @@ -0,0 +1,286 @@
>> +/*
>> + * net/sched/sch_cbs.c	Credit Based Shaper
>> + *
>> + *		This program is free software; you can redistribute it and/or
>> + *		modify it under the terms of the GNU General Public License
>> + *		as published by the Free Software Foundation; either version
>> + *		2 of the License, or (at your option) any later version.
>> + *
>> + * Authors:	Vininicius Costa Gomes <vinicius.gomes@intel.com>
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/types.h>
>> +#include <linux/kernel.h>
>> +#include <linux/string.h>
>> +#include <linux/errno.h>
>> +#include <linux/skbuff.h>
>> +#include <net/netlink.h>
>> +#include <net/sch_generic.h>
>> +#include <net/pkt_sched.h>
>> +
>> +struct cbs_sched_data {
>> +	struct Qdisc *qdisc; /* Inner qdisc, default - pfifo queue */
>> +	s32 queue;
>> +	s32 locredit;
>> +	s32 hicredit;
>> +	s32 sendslope;
>> +	s32 idleslope;
>> +};
>> +
>> +static int cbs_enqueue(struct sk_buff *skb, struct Qdisc *sch,
>> +		       struct sk_buff **to_free)
>> +{
>> +	struct cbs_sched_data *q = qdisc_priv(sch);
>> +	int ret;
>> +
>> +	ret = qdisc_enqueue(skb, q->qdisc, to_free);
>> +	if (ret != NET_XMIT_SUCCESS) {
>> +		if (net_xmit_drop_count(ret))
>> +			qdisc_qstats_drop(sch);
>> +		return ret;
>> +	}
>> +
>> +	qdisc_qstats_backlog_inc(sch, skb);
>> +	sch->q.qlen++;
>> +	return NET_XMIT_SUCCESS;
>> +}
>> +
>> +static struct sk_buff *cbs_dequeue(struct Qdisc *sch)
>> +{
>> +	struct cbs_sched_data *q = qdisc_priv(sch);
>> +	struct sk_buff *skb;
>> +
>> +	skb = q->qdisc->ops->peek(q->qdisc);
>> +	if (skb) {
>> +		skb = qdisc_dequeue_peeked(q->qdisc);
>> +		if (unlikely(!skb))
>> +			return NULL;
>> +
>> +		qdisc_qstats_backlog_dec(sch, skb);
>> +		sch->q.qlen--;
>> +		qdisc_bstats_update(sch, skb);
>> +
>> +		return skb;
>> +	}
>> +	return NULL;
>> +}
>> +
>> +static void cbs_reset(struct Qdisc *sch)
>> +{
>> +	struct cbs_sched_data *q = qdisc_priv(sch);
>> +
>> +	qdisc_reset(q->qdisc);
>> +}
>> +
>> +static const struct nla_policy cbs_policy[TCA_CBS_MAX + 1] = {
>> +	[TCA_CBS_PARMS]	= { .len = sizeof(struct tc_cbs_qopt) },
>> +};
>> +
>> +static int cbs_change(struct Qdisc *sch, struct nlattr *opt)
>> +{
>> +	struct cbs_sched_data *q = qdisc_priv(sch);
>> +	struct tc_cbs_qopt_offload cbs = { };
>> +	struct nlattr *tb[TCA_CBS_MAX + 1];
>> +	const struct net_device_ops *ops;
>> +	struct tc_cbs_qopt *qopt;
>> +	struct net_device *dev;
>> +	int err;
>> +
>> +	err = nla_parse_nested(tb, TCA_CBS_MAX, opt, cbs_policy, NULL);
>> +	if (err < 0)
>> +		return err;
>> +
>> +	err = -EINVAL;
>> +	if (!tb[TCA_CBS_PARMS])
>> +		goto done;
>> +
>> +	qopt = nla_data(tb[TCA_CBS_PARMS]);
>> +
>> +	dev = qdisc_dev(sch);
>> +	ops = dev->netdev_ops;
>> +
>> +	/* FIXME: this means that we can only install this qdisc
>> +	 * "under" mqprio. Do we need a more generic way to retrieve
>> +	 * the queue, or do we pass the netdev_queue to the driver?
>> +	 */
>> +	cbs.queue = TC_H_MIN(sch->parent) - 1 - netdev_get_num_tc(dev);
>> +
>> +	cbs.enable = 1;
>> +	cbs.hicredit = qopt->hicredit;
>> +	cbs.locredit = qopt->locredit;
>> +	cbs.idleslope = qopt->idleslope;
>> +	cbs.sendslope = qopt->sendslope;
>> +
>> +	err = -ENOTSUPP;
>> +	if (!ops->ndo_setup_tc)
>> +		goto done;
>
> This confuses tc a bit, and looking at other qdisc schedulers, it seems
> like EOPNOTSUPP is an alternative, this changes the output from
>
> RTNETLINK answers: Unknown error 524
>  - to
> RTNETLINK answers: Operation not supported
>

Yeah, I missed this. Thanks for catching this.

> which perhaps is more informative.
>
>> +
>> +	err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CBS, &cbs);
>> +	if (err < 0)
>> +		goto done;
>> +
>> +	q->queue = cbs.queue;
>> +	q->hicredit = cbs.hicredit;
>> +	q->locredit = cbs.locredit;
>> +	q->idleslope = cbs.idleslope;
>> +	q->sendslope = cbs.sendslope;
>> +
>> +done:
>> +	return err;
>> +}
>> +
>> +static int cbs_init(struct Qdisc *sch, struct nlattr *opt)
>> +{
>> +	struct cbs_sched_data *q = qdisc_priv(sch);
>> +
>> +	if (!opt)
>> +		return -EINVAL;
>> +
>> +	q->qdisc = fifo_create_dflt(sch, &pfifo_qdisc_ops, 1024);
>> +	qdisc_hash_add(q->qdisc, true);
>> +
>> +	return cbs_change(sch, opt);
>> +}
>> +
>> +static void cbs_destroy(struct Qdisc *sch)
>> +{
>> +	struct cbs_sched_data *q = qdisc_priv(sch);
>> +	struct tc_cbs_qopt_offload cbs = { };
>> +	struct net_device *dev;
>> +	int err;
>> +
>> +	q->hicredit = 0;
>> +	q->locredit = 0;
>> +	q->idleslope = 0;
>> +	q->sendslope = 0;
>> +
>> +	dev = qdisc_dev(sch);
>> +
>> +	cbs.queue = q->queue;
>> +	cbs.enable = 0;
>> +
>> +	err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CBS, &cbs);
>
> This can lead to NULL pointer deref if it is not set (tested for in
> cbs_change and error there leads us here, which then explodes).

Fixed.

>
>> +	if (err < 0)
>> +		pr_warn("Couldn't reset queue %d to default values\n",
>> +			cbs.queue);
>> +
>> +	qdisc_destroy(q->qdisc);
>
> Same, q->qdisc was NULL when cbs_init() failed.
>

Fixed the error path, thanks.


Cheers,

^ permalink raw reply

* [PATCH] net: ipv4: fix l3slave check for index returned in IP_PKTINFO
From: David Ahern @ 2017-09-14  0:11 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern

rt_iif is only set to the actual egress device for the output path. The
recent change to consider the l3slave flag when returning IP_PKTINFO
works for local traffic (the correct device index is returned), but it
broke the more typical use case of packets received from a remote host
always returning the VRF index rather than the original ingress device.
Update the fixup to consider l3slave and rt_iif actually getting set.

Fixes: 1dfa76390bf05 ("net: ipv4: add check for l3slave for index returned in IP_PKTINFO")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv4/ip_sockglue.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index e558e4f9597b..a599aa83fdad 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1207,7 +1207,6 @@ static int do_ip_setsockopt(struct sock *sk, int level,
 void ipv4_pktinfo_prepare(const struct sock *sk, struct sk_buff *skb)
 {
 	struct in_pktinfo *pktinfo = PKTINFO_SKB_CB(skb);
-	bool l3slave = ipv4_l3mdev_skb(IPCB(skb)->flags);
 	bool prepare = (inet_sk(sk)->cmsg_flags & IP_CMSG_PKTINFO) ||
 		       ipv6_sk_rxinfo(sk);
 
@@ -1221,8 +1220,13 @@ void ipv4_pktinfo_prepare(const struct sock *sk, struct sk_buff *skb)
 		 * (e.g., process binds socket to eth0 for Tx which is
 		 * redirected to loopback in the rtable/dst).
 		 */
-		if (pktinfo->ipi_ifindex == LOOPBACK_IFINDEX || l3slave)
+		struct rtable *rt = skb_rtable(skb);
+		bool l3slave = ipv4_l3mdev_skb(IPCB(skb)->flags);
+
+		if (pktinfo->ipi_ifindex == LOOPBACK_IFINDEX)
 			pktinfo->ipi_ifindex = inet_iif(skb);
+		else if (l3slave && rt && rt->rt_iif)
+			pktinfo->ipi_ifindex = rt->rt_iif;
 
 		pktinfo->ipi_spec_dst.s_addr = fib_compute_spec_dst(skb);
 	} else {
-- 
2.1.4

^ permalink raw reply related

* RE: [PATCH] dt-bindings: net: renesas-ravb: Add support for R8A77995 RAVB
From: Yoshihiro Shimoda @ 2017-09-14  0:11 UTC (permalink / raw)
  To: Sergei Shtylyov, davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1f4763ea-9f8a-03a6-1215-69b96c404590-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1568 bytes --]

Hello!

> From: Sergei Shtylyov
> Sent: Thursday, September 14, 2017 1:02 AM
> 
> Hello!
> 
> On 09/13/2017 03:17 PM, Yoshihiro Shimoda wrote:
> 
> > Add a new compatible string for the R8A77995 (R-Car D3) RAVB.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >   Documentation/devicetree/bindings/net/renesas,ravb.txt | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/renesas,ravb.txt
> b/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > index 1672353..ae2213f 100644
> > --- a/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > +++ b/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > @@ -17,6 +17,7 @@ Required properties:
> >
> >         - "renesas,etheravb-r8a7795" for the R8A7795 SoC.
> >         - "renesas,etheravb-r8a7796" for the R8A7796 SoC.
> > +      - "renesas,etheravb-r8a77995" for the R8A77995 SoC.
> 
>     That would conflict with the R8A77970 bindings patch posted by me
> yesterday. Please respin atop of it.

Yes, I submitted v2 patch now.

> >         - "renesas,etheravb-rcar-gen3" as a fallback for the above
> >   		R-Car Gen3 devices.
> >
> 
>     One more fragment is needed here, about the mandatory 'interrupt-names"
> prop. My patch makes this change unnecessary, however...

I understood it.

Best regards,
Yoshihiro Shimoda

> MBR, Sergei
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i

^ permalink raw reply

* [PATCH] dt-bindings: net: renesas-ravb: Add support for R8A77995 RAVB
From: Yoshihiro Shimoda @ 2017-09-14  0:06 UTC (permalink / raw)
  To: davem, robh+dt, mark.rutland
  Cc: netdev, linux-renesas-soc, devicetree, Yoshihiro Shimoda

Add a new compatible string for the R8A77995 (R-Car D3) RAVB.

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 Changes from v1:
  - Based on r8a77970 patch.
   https://marc.info/?l=linux-renesas-soc&m=150524655515476
  - Add Acked-by (Thanks Geert-san!).

 Documentation/devicetree/bindings/net/renesas,ravb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/renesas,ravb.txt b/Documentation/devicetree/bindings/net/renesas,ravb.txt
index 2689211..c902261 100644
--- a/Documentation/devicetree/bindings/net/renesas,ravb.txt
+++ b/Documentation/devicetree/bindings/net/renesas,ravb.txt
@@ -18,6 +18,7 @@ Required properties:
       - "renesas,etheravb-r8a7795" for the R8A7795 SoC.
       - "renesas,etheravb-r8a7796" for the R8A7796 SoC.
       - "renesas,etheravb-r8a77970" for the R8A77970 SoC.
+      - "renesas,etheravb-r8a77995" for the R8A77995 SoC.
       - "renesas,etheravb-rcar-gen3" as a fallback for the above
 		R-Car Gen3 devices.
 
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH v2 net] sctp: potential read out of bounds in sctp_ulpevent_type_enabled()
From: David Miller @ 2017-09-14  0:00 UTC (permalink / raw)
  To: dan.carpenter; +Cc: vyasevich, nhorman, linux-sctp, netdev, kernel-janitors
In-Reply-To: <20170913230054.fmtidvfi2swvy2mm@mwanda>

From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Thu, 14 Sep 2017 02:00:54 +0300

> This code causes a static checker warning because Smatch doesn't trust
> anything that comes from skb->data.  I've reviewed this code and I do
> think skb->data can be controlled by the user here.
> 
> The sctp_event_subscribe struct has 13 __u8 fields and we want to see
> if ours is non-zero.  sn_type can be any value in the 0-USHRT_MAX range.
> We're subtracting SCTP_SN_TYPE_BASE which is 1 << 15 so we could read
> either before the start of the struct or after the end.
> 
> This is a very old bug and it's surprising that it would go undetected
> for so long but my theory is that it just doesn't have a big impact so
> it would be hard to notice.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2:  Use reverse-christmas-tree local variable ordering.

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Job Offer In USA
From: Valero Energy Recruitment @ 2017-09-13 16:24 UTC (permalink / raw)
  To: Recipients

Valero Energy Oil & Gas Company USA
Employment Opportunity in Texas, USA.
Oil & Gas Sector ( NEW PLANT RIG )
Salary Range $7,000 USD - $11,700 Per Month
Kindly send your CV/RESUME for more details regarding our Job Program, Employment offers for all Offices.

CC: Forward All Documents/Certificates & Resume to: valeroenergy.recruit@gmail.com

^ permalink raw reply

* Re: [PATCH v2] net: smsc911x: Quieten netif during suspend
From: Florian Fainelli @ 2017-09-13 23:28 UTC (permalink / raw)
  To: Geert Uytterhoeven, David S . Miller, Steve Glendinning
  Cc: Andrew Lunn, netdev, linux-pm, linux-renesas-soc, linux-kernel
In-Reply-To: <1505324525-9998-1-git-send-email-geert+renesas@glider.be>



On 09/13/2017 10:42 AM, Geert Uytterhoeven wrote:
> If the network interface is kept running during suspend, the net core
> may call net_device_ops.ndo_start_xmit() while the Ethernet device is
> still suspended, which may lead to a system crash.
> 
> E.g. on sh73a0/kzm9g and r8a73a4/ape6evm, the external Ethernet chip is
> driven by a PM controlled clock.  If the Ethernet registers are accessed
> while the clock is not running, the system will crash with an imprecise
> external abort.
> 
> As this is a race condition with a small time window, it is not so easy
> to trigger at will.  Using pm_test may increase your chances:
> 
>     # echo 0 > /sys/module/printk/parameters/console_suspend
>     # echo platform > /sys/power/pm_test
>     # echo mem > /sys/power/state
> 
> To fix this, make sure the network interface is quietened during
> suspend.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

You may want to take the opportunity to suspend the PHY device
(conversely resume it) if WoL is not enabled on this device.

Thanks!

> ---
> This is v2 of the series "[PATCH 0/2] net: Fix crashes due to activity
> during suspend", which degenerated into a single patch after commit
> ebc8254aeae34226 ("Revert "net: phy: Correctly process PHY_HALTED in
> phy_stop_machine()"") made "[PATCH 1/2] net: phy: Freeze PHY polling before
> suspending devices" no longer needed.
> 
> v2:
>   - Spelling s/quit/quiet/g.
> 
> No stacktrace is provided, as the imprecise external abort is usually
> reported from an innocent looking and unrelated function like
> __loop_delay(), cpu_idle_poll(), or arch_timer_read_counter_long().
> ---
>  drivers/net/ethernet/smsc/smsc911x.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
> index 0b6a39b003a4e188..012fb66eed8dd618 100644
> --- a/drivers/net/ethernet/smsc/smsc911x.c
> +++ b/drivers/net/ethernet/smsc/smsc911x.c
> @@ -2595,6 +2595,11 @@ static int smsc911x_suspend(struct device *dev)
>  	struct net_device *ndev = dev_get_drvdata(dev);
>  	struct smsc911x_data *pdata = netdev_priv(ndev);
>  
> +	if (netif_running(ndev)) {
> +		netif_stop_queue(ndev);
> +		netif_device_detach(ndev);
> +	}
> +
>  	/* enable wake on LAN, energy detection and the external PME
>  	 * signal. */
>  	smsc911x_reg_write(pdata, PMT_CTRL,
> @@ -2628,7 +2633,15 @@ static int smsc911x_resume(struct device *dev)
>  	while (!(smsc911x_reg_read(pdata, PMT_CTRL) & PMT_CTRL_READY_) && --to)
>  		udelay(1000);
>  
> -	return (to == 0) ? -EIO : 0;
> +	if (to == 0)
> +		return -EIO;
> +
> +	if (netif_running(ndev)) {
> +		netif_device_attach(ndev);
> +		netif_start_queue(ndev);
> +	}
> +
> +	return 0;
>  }
>  
>  static const struct dev_pm_ops smsc911x_pm_ops = {
> 

-- 
Florian

^ permalink raw reply

* [PATCH v2 net] sctp: potential read out of bounds in sctp_ulpevent_type_enabled()
From: Dan Carpenter @ 2017-09-13 23:00 UTC (permalink / raw)
  To: Vlad Yasevich
  Cc: Neil Horman, David S. Miller, linux-sctp, netdev, kernel-janitors
In-Reply-To: <20170913.092522.934509429497822082.davem@davemloft.net>

This code causes a static checker warning because Smatch doesn't trust
anything that comes from skb->data.  I've reviewed this code and I do
think skb->data can be controlled by the user here.

The sctp_event_subscribe struct has 13 __u8 fields and we want to see
if ours is non-zero.  sn_type can be any value in the 0-USHRT_MAX range.
We're subtracting SCTP_SN_TYPE_BASE which is 1 << 15 so we could read
either before the start of the struct or after the end.

This is a very old bug and it's surprising that it would go undetected
for so long but my theory is that it just doesn't have a big impact so
it would be hard to notice.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2:  Use reverse-christmas-tree local variable ordering.

diff --git a/include/net/sctp/ulpevent.h b/include/net/sctp/ulpevent.h
index 1060494ac230..b8c86ec1a8f5 100644
--- a/include/net/sctp/ulpevent.h
+++ b/include/net/sctp/ulpevent.h
@@ -153,8 +153,12 @@ __u16 sctp_ulpevent_get_notification_type(const struct sctp_ulpevent *event);
 static inline int sctp_ulpevent_type_enabled(__u16 sn_type,
 					     struct sctp_event_subscribe *mask)
 {
+	int offset = sn_type - SCTP_SN_TYPE_BASE;
 	char *amask = (char *) mask;
-	return amask[sn_type - SCTP_SN_TYPE_BASE];
+
+	if (offset >= sizeof(struct sctp_event_subscribe))
+		return 0;
+	return amask[offset];
 }
 
 /* Given an event subscription, is this event enabled? */

^ permalink raw reply related

* Re: [PATCH] net: Convert int functions to bool
From: Joe Perches @ 2017-09-13 22:55 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: netdev
In-Reply-To: <20170913223158.GA7860@avx2>

On Thu, 2017-09-14 at 01:31 +0300, Alexey Dobriyan wrote:
> > Global function ipv6_rcv_saddr_equal and static functions
> > ipv6_rcv_saddr_equal and ipv4_rcv_saddr_equal currently return int.
> > 
> > bool is slightly more descriptive for these functions so change
> > their return type from int to bool.
> 
> From code generation POV "int" is better for non-inlined functions
> especially on non-x86.

I don't agree with you here.  For instance:

$ size net/ipv4/*.o*
   text	   data	    bss	    dec	    hex	filename
   9832	     26	      0	   9858	   2682	net/ipv4/inet_connection_sock.o.arm.defconfig.new
   9860	     26	      0	   9886	   269e	net/ipv4/inet_connection_sock.o.arm.defconfig.old

^ permalink raw reply

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
From: Cole Robinson @ 2017-09-13 22:49 UTC (permalink / raw)
  To: Josef Bacik, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <E068C58A-DC80-4D98-ADE6-8F05304EC7ED@fb.com>

On 09/13/2017 03:44 PM, Josef Bacik wrote:
> Alright thanks, this should fix it.
> 

Still no luck with all three patches applied to fedora 4.12.8-300 RPM. Pretty
sure I didn't mess up the testing but since I rarely do kernel builds it's not
impossible...

Thanks,
Cole

^ permalink raw reply

* Re: [PATCH] net: Convert int functions to bool
From: Alexey Dobriyan @ 2017-09-13 22:31 UTC (permalink / raw)
  To: Joe Perches; +Cc: netdev

> Global function ipv6_rcv_saddr_equal and static functions
> ipv6_rcv_saddr_equal and ipv4_rcv_saddr_equal currently return int.
>
> bool is slightly more descriptive for these functions so change
> their return type from int to bool.

>From code generation POV "int" is better for non-inlined functions
especially on non-x86.

Patch bloats even x86_64:

	$ ./scripts/bloat-o-meter ../vmlinux-002-do-while ../obj/vmlinux
	add/remove: 0/0 grow/shrink: 2/0 up/down: 3/0 (3)
	function                                     old     new   delta
	inet_rcv_saddr_equal                          38      40      +2
	inet_csk_get_port                           1300    1301      +1
	Total: Before=6084708, After=6084711, chg +0.00%

^ permalink raw reply

* Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates
From: Franklin S Cooper Jr @ 2017-09-13 21:58 UTC (permalink / raw)
  To: wg, mkl, mario.huettel, socketcan, quentin.schulz, edumazet,
	linux-can, netdev, linux-kernel, Nori, Sekhar
In-Reply-To: <20170818193947.27862-1-fcooper@ti.com>



On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote:
> During test transmitting using CAN-FD at high bitrates (4 Mbps) only
> resulted in errors. Scoping the signals I noticed that only a single bit
> was being transmitted and with a bit more investigation realized the actual
> MCAN IP would go back to initialization mode automatically.
> 
> It appears this issue is due to the MCAN needing to use the Transmitter
> Delay Compensation Mode as defined in the MCAN User's Guide. When this
> mode is used the User's Guide indicates that the Transmitter Delay
> Compensation Offset register should be set. The document mentions that this
> register should be set to (1/dbitrate)/2*(Func Clk Freq).
> 
> Additional CAN-CIA's "Bit Time Requirements for CAN FD" document indicates
> that this TDC mode is only needed for data bit rates above 2.5 Mbps.
> Therefore, only enable this mode and only set TDCO when the data bit rate
> is above 2.5 Mbps.
> 
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> ---
> I'm pretty surprised that this hasn't been implemented already since
> the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
> supports up to 10 Mbps.
> 
> So it will be nice to get comments from users of this driver to understand
> if they have been able to use CAN-FD beyond 2.5 Mbps without this patch.
> If they haven't what did they do to get around it if they needed higher
> speeds.
> 
> Meanwhile I plan on testing this using a more "realistic" CAN bus to insure
> everything still works at 5 Mbps which is the max speed of my CAN
> transceiver.

ping. Anyone has any thoughts on this?
> 
>  drivers/net/can/m_can/m_can.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index f4947a7..720e073 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -126,6 +126,12 @@ enum m_can_mram_cfg {
>  #define DBTP_DSJW_SHIFT		0
>  #define DBTP_DSJW_MASK		(0xf << DBTP_DSJW_SHIFT)
>  
> +/* Transmitter Delay Compensation Register (TDCR) */
> +#define TDCR_TDCO_SHIFT		8
> +#define TDCR_TDCO_MASK		(0x7F << TDCR_TDCO_SHIFT)
> +#define TDCR_TDCF_SHIFT		0
> +#define TDCR_TDCF_MASK		(0x7F << TDCR_TDCO_SHIFT)
> +
>  /* Test Register (TEST) */
>  #define TEST_LBCK		BIT(4)
>  
> @@ -977,6 +983,8 @@ static int m_can_set_bittiming(struct net_device *dev)
>  	const struct can_bittiming *dbt = &priv->can.data_bittiming;
>  	u16 brp, sjw, tseg1, tseg2;
>  	u32 reg_btp;
> +	u32 enable_tdc = 0;
> +	u32 tdco;
>  
>  	brp = bt->brp - 1;
>  	sjw = bt->sjw - 1;
> @@ -991,9 +999,23 @@ static int m_can_set_bittiming(struct net_device *dev)
>  		sjw = dbt->sjw - 1;
>  		tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
>  		tseg2 = dbt->phase_seg2 - 1;
> +
> +		/* TDC is only needed for bitrates beyond 2.5 MBit/s
> +		 * Specified in the "Bit Time Requirements for CAN FD" document
> +		 */
> +		if (dbt->bitrate > 2500000) {
> +			enable_tdc = DBTP_TDC;
> +			/* Equation based on Bosch's M_CAN User Manual's
> +			 * Transmitter Delay Compensation Section
> +			 */
> +			tdco = priv->can.clock.freq / (dbt->bitrate * 2);
> +			m_can_write(priv, M_CAN_TDCR, tdco << TDCR_TDCO_SHIFT);
> +		}
> +
>  		reg_btp = (brp << DBTP_DBRP_SHIFT) | (sjw << DBTP_DSJW_SHIFT) |
>  			(tseg1 << DBTP_DTSEG1_SHIFT) |
> -			(tseg2 << DBTP_DTSEG2_SHIFT);
> +			(tseg2 << DBTP_DTSEG2_SHIFT) | enable_tdc;
> +
>  		m_can_write(priv, M_CAN_DBTP, reg_btp);
>  	}
>  
> 

^ permalink raw reply

* Re: [patch net] net: sched: fix use-after-free in tcf_action_destroy and tcf_del_walker
From: David Miller @ 2017-09-13 20:59 UTC (permalink / raw)
  To: jiri; +Cc: netdev, jhs, xiyou.wangcong, kubakici, mlxsw
In-Reply-To: <20170913205006.GD1981@nanopsycho>

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 13 Sep 2017 22:50:06 +0200

> Wed, Sep 13, 2017 at 06:34:28PM CEST, davem@davemloft.net wrote:
>>From: Jiri Pirko <jiri@resnulli.us>
>>Date: Wed, 13 Sep 2017 17:32:37 +0200
>>
>>> From: Jiri Pirko <jiri@mellanox.com>
>>> 
>>> Recent commit d7fb60b9cafb ("net_sched: get rid of tcfa_rcu") removed
>>> freeing in call_rcu, which changed already existing hard-to-hit
>>> race condition into 100% hit:
>>> 
>>> [  598.599825] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
>>> [  598.607782] IP: tcf_action_destroy+0xc0/0x140
>>> 
>>> Or:
>>> 
>>> [   40.858924] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
>>> [   40.862840] IP: tcf_generic_walker+0x534/0x820
>>> 
>>> Fix this by storing the ops and use them directly for module_put call.
>>> 
>>> Fixes: a85a970af265 ("net_sched: move tc_action into tcf_common")
>>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>>
>>Applied, thanks Jiri.
> 
> Oh, I forgot to mention, this would be nice to push to stable.

Ok, queued up.

^ permalink raw reply

* [PATCH] net: Convert int functions to bool
From: Joe Perches @ 2017-09-13 20:58 UTC (permalink / raw)
  To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI; +Cc: netdev, linux-kernel

Global function ipv6_rcv_saddr_equal and static functions
ipv6_rcv_saddr_equal and ipv4_rcv_saddr_equal currently return int.

bool is slightly more descriptive for these functions so change
their return type from int to bool.

Signed-off-by: Joe Perches <joe@perches.com>
---
 include/net/addrconf.h          |  4 ++--
 net/ipv4/inet_connection_sock.c | 36 ++++++++++++++++++------------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index f44ff2476758..87981cd63180 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -94,8 +94,8 @@ int __ipv6_get_lladdr(struct inet6_dev *idev, struct in6_addr *addr,
 		      u32 banned_flags);
 int ipv6_get_lladdr(struct net_device *dev, struct in6_addr *addr,
 		    u32 banned_flags);
-int inet_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2,
-			 bool match_wildcard);
+bool inet_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2,
+			  bool match_wildcard);
 void addrconf_join_solict(struct net_device *dev, const struct in6_addr *addr);
 void addrconf_leave_solict(struct inet6_dev *idev, const struct in6_addr *addr);
 
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 4089c013cb03..054b26295638 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -39,11 +39,11 @@ EXPORT_SYMBOL(inet_csk_timer_bug_msg);
  *                          IPV6_ADDR_ANY only equals to IPV6_ADDR_ANY,
  *                          and 0.0.0.0 equals to 0.0.0.0 only
  */
-static int ipv6_rcv_saddr_equal(const struct in6_addr *sk1_rcv_saddr6,
-				const struct in6_addr *sk2_rcv_saddr6,
-				__be32 sk1_rcv_saddr, __be32 sk2_rcv_saddr,
-				bool sk1_ipv6only, bool sk2_ipv6only,
-				bool match_wildcard)
+static bool ipv6_rcv_saddr_equal(const struct in6_addr *sk1_rcv_saddr6,
+				 const struct in6_addr *sk2_rcv_saddr6,
+				 __be32 sk1_rcv_saddr, __be32 sk2_rcv_saddr,
+				 bool sk1_ipv6only, bool sk2_ipv6only,
+				 bool match_wildcard)
 {
 	int addr_type = ipv6_addr_type(sk1_rcv_saddr6);
 	int addr_type2 = sk2_rcv_saddr6 ? ipv6_addr_type(sk2_rcv_saddr6) : IPV6_ADDR_MAPPED;
@@ -52,29 +52,29 @@ static int ipv6_rcv_saddr_equal(const struct in6_addr *sk1_rcv_saddr6,
 	if (addr_type == IPV6_ADDR_MAPPED && addr_type2 == IPV6_ADDR_MAPPED) {
 		if (!sk2_ipv6only) {
 			if (sk1_rcv_saddr == sk2_rcv_saddr)
-				return 1;
+				return true;
 			if (!sk1_rcv_saddr || !sk2_rcv_saddr)
 				return match_wildcard;
 		}
-		return 0;
+		return false;
 	}
 
 	if (addr_type == IPV6_ADDR_ANY && addr_type2 == IPV6_ADDR_ANY)
-		return 1;
+		return true;
 
 	if (addr_type2 == IPV6_ADDR_ANY && match_wildcard &&
 	    !(sk2_ipv6only && addr_type == IPV6_ADDR_MAPPED))
-		return 1;
+		return true;
 
 	if (addr_type == IPV6_ADDR_ANY && match_wildcard &&
 	    !(sk1_ipv6only && addr_type2 == IPV6_ADDR_MAPPED))
-		return 1;
+		return true;
 
 	if (sk2_rcv_saddr6 &&
 	    ipv6_addr_equal(sk1_rcv_saddr6, sk2_rcv_saddr6))
-		return 1;
+		return true;
 
-	return 0;
+	return false;
 }
 #endif
 
@@ -82,20 +82,20 @@ static int ipv6_rcv_saddr_equal(const struct in6_addr *sk1_rcv_saddr6,
  * match_wildcard == false: addresses must be exactly the same, i.e.
  *                          0.0.0.0 only equals to 0.0.0.0
  */
-static int ipv4_rcv_saddr_equal(__be32 sk1_rcv_saddr, __be32 sk2_rcv_saddr,
-				bool sk2_ipv6only, bool match_wildcard)
+static bool ipv4_rcv_saddr_equal(__be32 sk1_rcv_saddr, __be32 sk2_rcv_saddr,
+				 bool sk2_ipv6only, bool match_wildcard)
 {
 	if (!sk2_ipv6only) {
 		if (sk1_rcv_saddr == sk2_rcv_saddr)
-			return 1;
+			return true;
 		if (!sk1_rcv_saddr || !sk2_rcv_saddr)
 			return match_wildcard;
 	}
-	return 0;
+	return false;
 }
 
-int inet_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2,
-			 bool match_wildcard)
+bool inet_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2,
+			  bool match_wildcard)
 {
 #if IS_ENABLED(CONFIG_IPV6)
 	if (sk->sk_family == AF_INET6)
-- 
2.10.0.rc2.1.g053435c

^ permalink raw reply related

* Re: [patch net] net: sched: fix use-after-free in tcf_action_destroy and tcf_del_walker
From: Jiri Pirko @ 2017-09-13 20:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, jhs, xiyou.wangcong, kubakici, mlxsw
In-Reply-To: <20170913.093428.806129519464331602.davem@davemloft.net>

Wed, Sep 13, 2017 at 06:34:28PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Wed, 13 Sep 2017 17:32:37 +0200
>
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Recent commit d7fb60b9cafb ("net_sched: get rid of tcfa_rcu") removed
>> freeing in call_rcu, which changed already existing hard-to-hit
>> race condition into 100% hit:
>> 
>> [  598.599825] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
>> [  598.607782] IP: tcf_action_destroy+0xc0/0x140
>> 
>> Or:
>> 
>> [   40.858924] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
>> [   40.862840] IP: tcf_generic_walker+0x534/0x820
>> 
>> Fix this by storing the ops and use them directly for module_put call.
>> 
>> Fixes: a85a970af265 ("net_sched: move tc_action into tcf_common")
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>Applied, thanks Jiri.

Oh, I forgot to mention, this would be nice to push to stable.

^ permalink raw reply

* Re: [PATCH] MAINTAINERS: review Renesas DT bindings as well
From: David Miller @ 2017-09-13 20:41 UTC (permalink / raw)
  To: sergei.shtylyov; +Cc: netdev, linux-renesas-soc, linux-kernel
In-Reply-To: <20170913192913.138491844@cogentembedded.com>

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Wed, 13 Sep 2017 22:28:53 +0300

> When adding myself  as a reviewer for  the Renesas  Ethernet drivers
> I somehow forgot about the bindings -- I want to review them as well.
> 
> Fixes: 8e6569af3a1b ("MAINTAINERS: add myself as Renesas Ethernet drivers reviewer")
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Applied.

^ permalink raw reply

* Re: [PATCH net] net_sched: gen_estimator: fix scaling error in bytes/packets samples
From: David Miller @ 2017-09-13 20:34 UTC (permalink / raw)
  To: eric.dumazet; +Cc: nuclearcat, netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <1505326605.15310.193.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 13 Sep 2017 11:16:45 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> Denys reported wrong rate estimations with HTB classes.
> 
> It appears the bug was added in linux-4.10, since my tests
> where using intervals of one second only.
> 
> HTB using 4 sec default rate estimators, reported rates
> were 4x higher.
> 
> We need to properly scale the bytes/packets samples before
> integrating them in EWMA.
> 
> Tested:
 ...
> Fixes: 1c0d32fde5bd ("net_sched: gen_estimator: complete rewrite of rate estimators")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Denys Fedoryshchenko <nuclearcat@nuclearcat.com>

Applied and queued up for -stable, thanks Eric.

^ permalink raw reply

* Re: [PATCH v2] geneve: Fix setting ttl value in collect metadata mode
From: Pravin Shelar @ 2017-09-13 20:22 UTC (permalink / raw)
  To: 严海双
  Cc: David S. Miller, Girish Moodalbail,
	Linux Kernel Network Developers, linux-kernel
In-Reply-To: <95A24784-0575-47CE-A485-B2A59911262A@cmss.chinamobile.com>

On Wed, Sep 13, 2017 at 4:15 AM, 严海双 <yanhaishuang@cmss.chinamobile.com> wrote:
>
>
>> On 2017年9月13日, at 上午7:43, Pravin Shelar <pshelar@ovn.org> wrote:
>>
>> On Tue, Sep 12, 2017 at 12:05 AM, Haishuang Yan
>> <yanhaishuang@cmss.chinamobile.com> wrote:
>>> Similar to vxlan/ipip tunnel, if key->tos is zero in collect metadata
>>> mode, tos should also fallback to ip{4,6}_dst_hoplimit.
>>>
>>> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
>>>
>>> ---
>>> Changes since v2:
>>>  * Make the commit message more clearer.
>>> ---
>>> drivers/net/geneve.c | 6 ++----
>>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
>>> index f640407..d52a65f 100644
>>> --- a/drivers/net/geneve.c
>>> +++ b/drivers/net/geneve.c
>>> @@ -834,11 +834,10 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>>>        sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
>>>        if (geneve->collect_md) {
>>>                tos = ip_tunnel_ecn_encap(key->tos, ip_hdr(skb), skb);
>>> -               ttl = key->ttl;
>>>        } else {
>>>                tos = ip_tunnel_ecn_encap(fl4.flowi4_tos, ip_hdr(skb), skb);
>>> -               ttl = key->ttl ? : ip4_dst_hoplimit(&rt->dst);
>>>        }
>>> +       ttl = key->ttl ? : ip4_dst_hoplimit(&rt->dst);
>>>        df = key->tun_flags & TUNNEL_DONT_FRAGMENT ? htons(IP_DF) : 0;
>>>
>> This changes user API of Geneve collect-metadata mode. I do not see
>> good reason for this. Why user can not set right TTL for the flow?
>>
>
> For example, I test this case with script samples/bpf/test_tunnel_bpf.sh,
> and modify samples/bpf/tcbpf2_kern.c with following patch:
>
But user is suppose to specify the TTL in collect-md mode for Geneve
tunnel. That is the API.

^ permalink raw reply

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
From: Josef Bacik @ 2017-09-13 20:11 UTC (permalink / raw)
  To: Chuck Ebbert
  Cc: Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Cole Robinson
In-Reply-To: <20170913154750.100881c0@gmail.com>


> On Sep 13, 2017, at 12:46 PM, Chuck Ebbert <cebbert.lkml@gmail.com> wrote:
> 
> On Wed, 13 Sep 2017 17:28:25 +0000
> Josef Bacik <jbacik@fb.com> wrote:
> 
>> Sorry I thought I had made this other fix, can you apply this on top
>> of the other one and try that?  I have more things to try if this
>> doesn’t work, sorry you are playing go between, but I want to make
>> sure I know _which_ fix actually fixes the problem, and then clean up
>> in followup patches.  Thanks,
>> 
>> Josef
>> 
>> On 9/13/17, 8:45 AM, "Laura Abbott" <labbott@redhat.com> wrote:
>> 
>> On 09/12/2017 04:12 PM, Josef Bacik wrote:
>>> First I’m super sorry for the top post, I’m at plumbers and I
>>> forgot to upload my muttrc to my new cloud instance, so I’m screwed
>>> using outlook.
>>> 
>>> I have a completely untested, uncompiled patch that I think will
>>> fix the problem, would you mind giving it a go?  Thanks,
>>> 
>>> Josef  
>> 
>> Thanks for the quick turnaround. Unfortunately, the problem is still
>> reproducible according to the reporter.
>> 
>> Thanks,
>> Laura
> 
> I am confused by the patch that originally caused this:
> 
>        if (sk->sk_family == AF_INET6)
>                return ipv6_rcv_saddr_equal(&sk->sk_v6_rcv_saddr,
> -                                           &sk2->sk_v6_rcv_saddr,
> +                                           inet6_rcv_saddr(sk2),
>                                            sk->sk_rcv_saddr,
>                                            sk2->sk_rcv_saddr,
> 
> Shouldn't the first argument also be changed to use inet6_rcv_saddr()?

No we know sk is IPv6 so it's alright to use directly.  Thanks,

Josef

^ permalink raw reply

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
From: Chuck Ebbert @ 2017-09-13 19:47 UTC (permalink / raw)
  To: Josef Bacik
  Cc: Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Cole Robinson
In-Reply-To: <EF55EF02-AD40-4F06-8C07-4F9F4221BFE6@fb.com>

On Wed, 13 Sep 2017 17:28:25 +0000
Josef Bacik <jbacik@fb.com> wrote:

> Sorry I thought I had made this other fix, can you apply this on top
> of the other one and try that?  I have more things to try if this
> doesn’t work, sorry you are playing go between, but I want to make
> sure I know _which_ fix actually fixes the problem, and then clean up
> in followup patches.  Thanks,
> 
> Josef
> 
> On 9/13/17, 8:45 AM, "Laura Abbott" <labbott@redhat.com> wrote:
> 
> On 09/12/2017 04:12 PM, Josef Bacik wrote:
> > First I’m super sorry for the top post, I’m at plumbers and I
> > forgot to upload my muttrc to my new cloud instance, so I’m screwed
> > using outlook.
> > 
> > I have a completely untested, uncompiled patch that I think will
> > fix the problem, would you mind giving it a go?  Thanks,
> > 
> > Josef  
> 
> Thanks for the quick turnaround. Unfortunately, the problem is still
> reproducible according to the reporter.
> 
> Thanks,
> Laura

I am confused by the patch that originally caused this:

        if (sk->sk_family == AF_INET6)
                return ipv6_rcv_saddr_equal(&sk->sk_v6_rcv_saddr,
-                                           &sk2->sk_v6_rcv_saddr,
+                                           inet6_rcv_saddr(sk2),
                                            sk->sk_rcv_saddr,
                                            sk2->sk_rcv_saddr,

Shouldn't the first argument also be changed to use inet6_rcv_saddr()?

^ permalink raw reply

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
From: Josef Bacik @ 2017-09-13 19:44 UTC (permalink / raw)
  To: Cole Robinson, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <a4807126-1f17-20bd-dace-d9c6b6c4787b@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

Alright thanks, this should fix it.

Josef

On 9/13/17, 12:14 PM, "Cole Robinson" <crobinso@redhat.com> wrote:

On 09/13/2017 01:40 PM, Cole Robinson wrote:
> On 09/13/2017 01:28 PM, Josef Bacik wrote:
>> Sorry I thought I had made this other fix, can you apply this on top of the other one and try that?  I have more things to try if this doesn’t work, sorry you are playing go between, but I want to make sure I know _which_ fix actually fixes the problem, and then clean up in followup patches.  Thanks,
>>
> 
> I'm the bug reporter. I'll combine the two patches and report back
> 

Nope, issue is still present with both patches applied. Tried my own build and
a package Laura provided

Thanks,
Cole




[-- Attachment #2: 0001-net-don-t-fast-patch-mismatched-sockets-in-STRICT-mo.patch --]
[-- Type: application/octet-stream, Size: 1182 bytes --]

From 8d3da6d30a7c63034d4ff7d4fc33ea9e3f23cf41 Mon Sep 17 00:00:00 2001
From: Josef Bacik <josef@toxicpanda.com>
Date: Wed, 13 Sep 2017 12:42:13 -0700
Subject: [PATCH] net: don't fast patch mismatched sockets in STRICT mode

With FASTREUSE_STRICT we may have sockets on the tb that don't match our
fast_sk information, so if our new socket don't strictly match the
fast_sk info we need to not go down the fast path and do the
inet_csk_bind_conflict so we can look at the other sockets on the tb.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 net/ipv4/inet_connection_sock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index ff8b15a99e42..fe9cf4862de2 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -263,6 +263,8 @@ static inline int sk_reuseport_match(struct inet_bind_bucket *tb,
 	 */
 	if (tb->fastreuseport == FASTREUSEPORT_ANY)
 		return 1;
+	if (tb->fast_ipv6_only && tb->fast_sk_family != sk->sk_family)
+		return 0;
 #if IS_ENABLED(CONFIG_IPV6)
 	if (tb->fast_sk_family == AF_INET6)
 		return ipv6_rcv_saddr_equal(&tb->fast_v6_rcv_saddr,
-- 
2.13.5


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox