Netdev List
 help / color / mirror / Atom feed
* [PATCH] ipvs: SNAT packet replies only for NATed connections
From: Simon Horman @ 2017-05-08  9:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Simon Horman
In-Reply-To: <1494236923-8015-1-git-send-email-horms@verge.net.au>

From: Julian Anastasov <ja@ssi.bg>

We do not check if packet from real server is for NAT
connection before performing SNAT. This causes problems
for setups that use DR/TUN and allow local clients to
access the real server directly, for example:

- local client in director creates IPVS-DR/TUN connection
CIP->VIP and the request packets are routed to RIP.
Talks are finished but IPVS connection is not expired yet.

- second local client creates non-IPVS connection CIP->RIP
with same reply tuple RIP->CIP and when replies are received
on LOCAL_IN we wrongly assign them for the first client
connection because RIP->CIP matches the reply direction.
As result, IPVS SNATs replies for non-IPVS connections.

The problem is more visible to local UDP clients but in rare
cases it can happen also for TCP or remote clients when the
real server sends the reply traffic via the director.

So, better to be more precise for the reply traffic.
As replies are not expected for DR/TUN connections, better
to not touch them.

Reported-by: Nick Moriarty <nick.moriarty@york.ac.uk>
Tested-by: Nick Moriarty <nick.moriarty@york.ac.uk>
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_core.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index d2d7bdf1d510..ad99c1ceea6f 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -849,10 +849,8 @@ static int handle_response_icmp(int af, struct sk_buff *skb,
 {
 	unsigned int verdict = NF_DROP;
 
-	if (IP_VS_FWD_METHOD(cp) != 0) {
-		pr_err("shouldn't reach here, because the box is on the "
-		       "half connection in the tun/dr module.\n");
-	}
+	if (IP_VS_FWD_METHOD(cp) != IP_VS_CONN_F_MASQ)
+		goto ignore_cp;
 
 	/* Ensure the checksum is correct */
 	if (!skb_csum_unnecessary(skb) && ip_vs_checksum_complete(skb, ihl)) {
@@ -886,6 +884,8 @@ static int handle_response_icmp(int af, struct sk_buff *skb,
 		ip_vs_notrack(skb);
 	else
 		ip_vs_update_conntrack(skb, cp, 0);
+
+ignore_cp:
 	verdict = NF_ACCEPT;
 
 out:
@@ -1385,8 +1385,11 @@ ip_vs_out(struct netns_ipvs *ipvs, unsigned int hooknum, struct sk_buff *skb, in
 	 */
 	cp = pp->conn_out_get(ipvs, af, skb, &iph);
 
-	if (likely(cp))
+	if (likely(cp)) {
+		if (IP_VS_FWD_METHOD(cp) != IP_VS_CONN_F_MASQ)
+			goto ignore_cp;
 		return handle_response(af, skb, pd, cp, &iph, hooknum);
+	}
 
 	/* Check for real-server-started requests */
 	if (atomic_read(&ipvs->conn_out_counter)) {
@@ -1444,9 +1447,15 @@ ip_vs_out(struct netns_ipvs *ipvs, unsigned int hooknum, struct sk_buff *skb, in
 			}
 		}
 	}
+
+out:
 	IP_VS_DBG_PKT(12, af, pp, skb, iph.off,
 		      "ip_vs_out: packet continues traversal as normal");
 	return NF_ACCEPT;
+
+ignore_cp:
+	__ip_vs_conn_put(cp);
+	goto out;
 }
 
 /*
-- 
2.1.4


^ permalink raw reply related

* [GIT PULL 0/1] IPVS Fixes for v4.12
From: Simon Horman @ 2017-05-08  9:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Simon Horman

Hi Pablo,

please consider this fix to IPVS for v4.12.

* It is a fix from Julian Anastasov to only SNAT SNAT packet replies only for
  NATed connections


My understanding is that this fix is appropriate for 4.9.25, 4.10.13, 4.11
as well as the nf tree. Julian has separately posted backports for other
-stable kernels; please see:

* [PATCH 3.2.88,3.4.113 -stable 1/3] ipvs: SNAT packet replies only for
        NATed connections
* [PATCH 3.10.105,3.12.73,3.16.43,4.1.39 -stable 2/3] ipvs: SNAT packet
        replies only for NATed connections 
* [PATCH 4.4.65 -stable 3/3] ipvs: SNAT packet replies only for NATed
        connections


The following changes since commit f411af6822182f84834c4881b825dd40534e7fe8:

  Merge branch 'ibmvnic-Updated-reset-handler-andcode-fixes' (2017-05-03 11:33:06 -0400)

are available in the git repository at:

  http://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git tags/ipvs-fixes-for-v4.12

for you to fetch changes up to 3c5ab3f395d66a9e4e937fcfdf6ebc63894f028b:

  ipvs: SNAT packet replies only for NATed connections (2017-05-08 11:38:35 +0200)

----------------------------------------------------------------
Julian Anastasov (1):
      ipvs: SNAT packet replies only for NATed connections

 net/netfilter/ipvs/ip_vs_core.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

^ permalink raw reply

* Re: [PATCH v3 net-next 01/11] net: stmmac: prepare dma op mode config for multiple queues
From: Joao Pinto @ 2017-05-08  9:54 UTC (permalink / raw)
  To: Andy Shevchenko, Jan Kiszka
  Cc: Joao Pinto, David S. Miller, Giuseppe CAVALLARO, Alexandre TORGUE,
	netdev, Linux Kernel Mailing List
In-Reply-To: <CAHp75Vfe0pXTEvdt7Z3-DgMTFdC5B3106ubj_D+_ng0vNCUaRA@mail.gmail.com>

Hi Andy and Jan,

Às 10:36 AM de 5/8/2017, Andy Shevchenko escreveu:
> On Mon, May 8, 2017 at 9:56 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2017-03-15 12:04, Joao Pinto wrote:
>>> This patch prepares DMA Operation Mode configuration for multiple queues.
>>> The work consisted on breaking the DMA operation Mode configuration function
>>> into RX and TX scope and adapting its mechanism in stmmac_main.
> 
>> Starting with this patch, the stmmac-based network adapters of the Intel
>> Quark SoC stop working. I'm getting an IP via DHCP, I can ping, but TCP
>> connections can no longer be established.
>>
>> Moving on a few patches (didn't bisect the exact one yet), the TX
>> watchdog starts to fire, and DHCP fails completely. And if I go to
>> current master in Linus tree (reverting an unrelated boot regression), I
>> even get a crash in stmmac_xmit.
>>
>> Here are some details about the hw from dma_cap POV, if this helps:
>>
>> ==============================
>>         DMA HW features
>> ==============================
>>         10/100 Mbps: Y
>>         1000 Mbps: N
>>         Half duplex: Y
>>         Hash Filter: Y
>>         Multiple MAC address registers: N
>>         PCS (TBI/SGMII/RTBI PHY interfaces): N
>>         SMA (MDIO) Interface: Y
>>         PMT Remote wake up: N
>>         PMT Magic Frame: N
>>         RMON module: Y
>>         IEEE 1588-2002 Time Stamp: N
>>         IEEE 1588-2008 Advanced Time Stamp: Y
>>         802.3az - Energy-Efficient Ethernet (EEE): N
>>         AV features: N
>>         Checksum Offload in TX: Y
>>         IP Checksum Offload (type1) in RX: N
>>         IP Checksum Offload (type2) in RX: Y
>>         RXFIFO > 2048bytes: Y
>>         Number of Additional RX channel: 0
>>         Number of Additional TX channel: 0
>>         Enhanced descriptors: Y
>>
>> Given the number of different failure modes, my feeling is that there
>> are multiple regressions coming with these patches...
>>
>> I've tested on the IOT2000 board, but I suspect the Galileo Gen2 will be
>> affected equally. If you don't have access to any such device, let me
>> know what I can debug for you.
> 
> JFYI: With today's linux-next when _kexec:ed_ kernel boots I tried and
> got the following:
> 
> 
> # ip a s
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue qlen 1000
>    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>    inet 127.0.0.1/8 scope host lo
>       valid_lft forever preferred_lft forever
>    inet6 ::1/128 scope host
>       valid_lft forever preferre[  130.403995] random: fast init done
> d_lft forever
> 2: eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop qlen 1000
>    link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
> 3: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop qlen 1000
>    link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
> 4: sit0@NONE: <NOARP> mtu 1480 qdisc noop qlen 1000
>    link/sit 0.0.0.0 brd 0.0.0.0
> # udhcpc -i eth0
> udhcpc: started, v1.26.2
> [  140.825131] stmmaceth 0000:00:14.6 eth0: device MAC address 98:4f:ee:05:ac:47
> [  140.834304] Generic PHY stmmac-a6:01: attached PHY driver [Generic
> PHY] (mii_bus:phy_addr=stmmac-a6:01, irq=-1)
> [  140.930871] stmmaceth 0000:00:14.6 eth0: IEEE 1588-2008 Advanced
> Timestamp supported
> [  140.941109] stmmaceth 0000:00:14.6 eth0: registered PTP clock
> [  140.953626] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
> udhcpc: sending discover
> [  142.979557] stmmaceth 0000:00:14.6 eth0: Link is Up - 100Mbps/Full
> - flow control off
> [  142.988756] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> [  142.998810] BUG: unable to handle kernel NULL pointer dereference at   (null)
> [  143.006193] IP: stmmac_xmit+0xf1/0x1080
> [  143.010168] *pde = 00000000
> [  143.010177]
> [  143.014762] Oops: 0002 [#1]
> [  143.017672] Modules linked in: at24 nvmem_core pwm_pca9685
> [  143.023338] CPU: 0 PID: 0 Comm: swapper Not tainted 4.11.0-next-20170508+ #2
> [  143.030539] task: c8533580 task.stack: c852c000
> [  143.035237] EIP: stmmac_xmit+0xf1/0x1080
> [  143.039302] EFLAGS: 00010216 CPU: 0
> [  143.042915] EAX: 00000000 EBX: 00000050 ECX: 00000000 EDX: ceb6a0c0
> [  143.049326] ESI: 00000000 EDI: cdd16000 EBP: cdc25d70 ESP: cdc25d20
> [  143.055735]  DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
> [  143.061271] CR0: 80050033 CR2: 00000000 CR3: 0eb5c000 CR4: 00100010
> [  143.067671] Call Trace:
> [  143.070238]  <SOFTIRQ>
> [  143.072763]  dev_hard_start_xmit+0x7c/0x1a0
> [  143.077120]  sch_direct_xmit+0xf0/0x120
> [  143.081130]  __dev_queue_xmit+0x181/0x430
> [  143.085311]  ? eth_commit_mac_addr_change+0x20/0x20
> [  143.090362]  dev_queue_xmit+0xa/0x10
> [  143.094100]  neigh_resolve_output+0xdb/0x190
> [  143.098561]  ip6_finish_output2+0x184/0x500
> [  143.102945]  ip6_finish_output+0x91/0xe0
> [  143.107057]  ? ip6_finish_output+0x91/0xe0
> [  143.111338]  ip6_output+0x36/0x110
> [  143.114924]  ? ip6_fragment+0xb00/0xb00
> [  143.118935]  mld_sendpack+0x191/0x2b0
> [  143.122769]  ? mld_newpack+0xda/0x180
> [  143.126598]  ? ipv6_icmp_sysctl_init+0x30/0x30
> [  143.131224]  mld_ifc_timer_expire+0x158/0x240
> [  143.135756]  ? find_next_bit+0xa/0x10
> [  143.139584]  ? mld_dad_timer_expire+0x50/0x50
> [  143.144112]  call_timer_fn+0x2a/0xf0
> [  143.147862]  ? mld_dad_timer_expire+0x50/0x50
> [  143.152395]  run_timer_softirq+0x158/0x300
> [  143.156668]  ? file_free_rcu+0x1e/0x30
> [  143.160589]  __do_softirq+0xc4/0x200
> [  143.164341]  ? __hrtimer_tasklet_trampoline+0x30/0x30
> [  143.169575]  do_softirq_own_stack+0x1e/0x30
> [  143.173902]  </SOFTIRQ>
> [  143.176502]  irq_exit+0x95/0xa0
> [  143.179812]  smp_apic_timer_interrupt+0x31/0x40
> [  143.184530]  apic_timer_interrupt+0x32/0x40
> [  143.188889] EIP: default_idle+0xc/0x70
> [  143.192774] EFLAGS: 00000246 CPU: 0
> [  143.196386] EAX: 00000000 EBX: 00000000 ECX: 00000001 EDX: 00000000
> [  143.202795] ESI: 00000000 EDI: c8533580 EBP: c852df54 ESP: c852df4c
> [  143.209205]  DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
> [  143.214780]  arch_cpu_idle+0x9/0x10
> [  143.218446]  default_idle_call+0x17/0x30
> [  143.222551]  do_idle+0xed/0x130
> [  143.225873]  cpu_startup_entry+0x15/0x20
> [  143.229965]  rest_init+0x5c/0x60
> [  143.233370]  start_kernel+0x313/0x318
> [  143.237221]  i386_start_kernel+0x98/0x9c
> [  143.241315]  startup_32_smp+0x16b/0x16d
> [  143.245289] Code: 84 45 06 00 00 c1 e2 05 03 94 c7 9c 09 00 00 89
> 55 b0 8b 45 c8 8b 75 bc 8b 55 d8 8d 1c 80 89
> 75 e4 c1 e3 03 8b 84 1f a4 09 00 00 <89> 14 b0 8b 87 40 0d 00 00 8b 40
> 24 85 c0 89 45 b8 0f 85 68 02
> [  143.264746] EIP: stmmac_xmit+0xf1/0x1080 SS:ESP: 0068:cdc25d20
> [  143.270727] CR2: 0000000000000000
> [  143.274175] ---[ end trace 79da8ef70f8b98d7 ]---
> [  143.278925] Kernel panic - not syncing: Fatal exception in interrupt
> [  143.285433] Kernel Offset: 0x6a00000 from 0xc1000000 (relocation
> range: 0xc0000000-0xd05effff)
> [  143.294268] ---[ end Kernel panic - not syncing: Fatal exception in interrupt
> 
> 

Are you using the same version of Ethernet IP, 10/100?
Could you please verify if the crash you are experiencing is this place?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#n2956

I would say that for rather old IPs, the napi is not capable of giving a valid
queue number. Could you please print the queue index returned by this line?

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#n2948

Thank you.

Joao Pinto

^ permalink raw reply

* [PATCH v2 net] vti: check nla_put_* return value
From: Hangbin Liu @ 2017-05-08  9:57 UTC (permalink / raw)
  To: netdev; +Cc: Hangbin Liu
In-Reply-To: <1494225380-12322-1-git-send-email-liuhangbin@gmail.com>

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/ipv4/ip_vti.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 4097741..4ec9aff 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -546,12 +546,13 @@ static int vti_fill_info(struct sk_buff *skb, const struct net_device *dev)
 	struct ip_tunnel *t = netdev_priv(dev);
 	struct ip_tunnel_parm *p = &t->parms;
 
-	nla_put_u32(skb, IFLA_VTI_LINK, p->link);
-	nla_put_be32(skb, IFLA_VTI_IKEY, p->i_key);
-	nla_put_be32(skb, IFLA_VTI_OKEY, p->o_key);
-	nla_put_in_addr(skb, IFLA_VTI_LOCAL, p->iph.saddr);
-	nla_put_in_addr(skb, IFLA_VTI_REMOTE, p->iph.daddr);
-	nla_put_u32(skb, IFLA_VTI_FWMARK, t->fwmark);
+	if (nla_put_u32(skb, IFLA_VTI_LINK, p->link) ||
+	    nla_put_be32(skb, IFLA_VTI_IKEY, p->i_key) ||
+	    nla_put_be32(skb, IFLA_VTI_OKEY, p->o_key) ||
+	    nla_put_in_addr(skb, IFLA_VTI_LOCAL, p->iph.saddr) ||
+	    nla_put_in_addr(skb, IFLA_VTI_REMOTE, p->iph.daddr) ||
+	    nla_put_u32(skb, IFLA_VTI_FWMARK, t->fwmark))
+		return -EMSGSIZE;
 
 	return 0;
 }
-- 
2.5.5

^ permalink raw reply related

* [PATCH net] tcp: init tcp_options before using it.
From: Hangbin Liu @ 2017-05-08  9:57 UTC (permalink / raw)
  To: netdev; +Cc: Hangbin Liu

I searched 4308fc58dced ("tcp: Document use of undefined variable") in
archive list, but did not find the thread. So I'm not sure why we only
add a description about un-initialized value.

Even we don't use tmp_opt.sack_ok, I think it would be more safe to
initialize the value before using it. Just as other caller did.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/ipv4/tcp_minisocks.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 717be4d..6ca2546 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -85,8 +85,6 @@ tcp_timewait_check_oow_rate_limit(struct inet_timewait_sock *tw,
  * spinlock it. I do not want! Well, probability of misbehaviour
  * is ridiculously low and, seems, we could use some mb() tricks
  * to avoid misread sequence numbers, states etc.  --ANK
- *
- * We don't need to initialize tmp_out.sack_ok as we don't use the results
  */
 enum tcp_tw_status
 tcp_timewait_state_process(struct inet_timewait_sock *tw, struct sk_buff *skb,
@@ -96,7 +94,7 @@ tcp_timewait_state_process(struct inet_timewait_sock *tw, struct sk_buff *skb,
 	struct tcp_timewait_sock *tcptw = tcp_twsk((struct sock *)tw);
 	bool paws_reject = false;
 
-	tmp_opt.saw_tstamp = 0;
+	memset(&tmp_opt, 0, sizeof(tmp_opt));
 	if (th->doff > (sizeof(*th) >> 2) && tcptw->tw_ts_recent_stamp) {
 		tcp_parse_options(skb, &tmp_opt, 0, NULL);
 
@@ -542,8 +540,6 @@ EXPORT_SYMBOL(tcp_create_openreq_child);
  *
  * XXX (TFO) - The current impl contains a special check for ack
  * validation and inside tcp_v4_reqsk_send_ack(). Can we do better?
- *
- * We don't need to initialize tmp_opt.sack_ok as we don't use the results
  */
 
 struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
@@ -557,7 +553,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
 	bool paws_reject = false;
 	bool own_req;
 
-	tmp_opt.saw_tstamp = 0;
+	memset(&tmp_opt, 0, sizeof(tmp_opt));
 	if (th->doff > (sizeof(struct tcphdr)>>2)) {
 		tcp_parse_options(skb, &tmp_opt, 0, NULL);
 
-- 
2.5.5

^ permalink raw reply related

* Re: [PATCH v3 net-next 01/11] net: stmmac: prepare dma op mode config for multiple queues
From: Andy Shevchenko @ 2017-05-08 10:12 UTC (permalink / raw)
  To: Joao Pinto
  Cc: Jan Kiszka, David S. Miller, Giuseppe CAVALLARO, Alexandre TORGUE,
	netdev, Linux Kernel Mailing List
In-Reply-To: <96c988c8-2d4a-33df-0d20-cc8adac3b01c@synopsys.com>

On Mon, May 8, 2017 at 12:54 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
> Hi Andy and Jan,
>
> Às 10:36 AM de 5/8/2017, Andy Shevchenko escreveu:
>> On Mon, May 8, 2017 at 9:56 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2017-03-15 12:04, Joao Pinto wrote:
>>>> This patch prepares DMA Operation Mode configuration for multiple queues.
>>>> The work consisted on breaking the DMA operation Mode configuration function
>>>> into RX and TX scope and adapting its mechanism in stmmac_main.
>>
>>> Starting with this patch, the stmmac-based network adapters of the Intel
>>> Quark SoC stop working. I'm getting an IP via DHCP, I can ping, but TCP
>>> connections can no longer be established.

>> JFYI: With today's linux-next when _kexec:ed_ kernel boots I tried and
>> got the following:
>>
>>
>> # ip a s
>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue qlen 1000
>>    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>    inet 127.0.0.1/8 scope host lo
>>       valid_lft forever preferred_lft forever
>>    inet6 ::1/128 scope host
>>       valid_lft forever preferre[  130.403995] random: fast init done
>> d_lft forever
>> 2: eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop qlen 1000
>>    link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
>> 3: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop qlen 1000
>>    link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
>> 4: sit0@NONE: <NOARP> mtu 1480 qdisc noop qlen 1000
>>    link/sit 0.0.0.0 brd 0.0.0.0
>> # udhcpc -i eth0
>> udhcpc: started, v1.26.2
>> [  140.825131] stmmaceth 0000:00:14.6 eth0: device MAC address 98:4f:ee:05:ac:47
>> [  140.834304] Generic PHY stmmac-a6:01: attached PHY driver [Generic
>> PHY] (mii_bus:phy_addr=stmmac-a6:01, irq=-1)
>> [  140.930871] stmmaceth 0000:00:14.6 eth0: IEEE 1588-2008 Advanced
>> Timestamp supported
>> [  140.941109] stmmaceth 0000:00:14.6 eth0: registered PTP clock
>> [  140.953626] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
>> udhcpc: sending discover
>> [  142.979557] stmmaceth 0000:00:14.6 eth0: Link is Up - 100Mbps/Full
>> - flow control off
>> [  142.988756] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
>> [  142.998810] BUG: unable to handle kernel NULL pointer dereference at   (null)
>> [  143.006193] IP: stmmac_xmit+0xf1/0x1080
>> [  143.010168] *pde = 00000000
>> [  143.010177]
>> [  143.014762] Oops: 0002 [#1]
>> [  143.017672] Modules linked in: at24 nvmem_core pwm_pca9685
>> [  143.023338] CPU: 0 PID: 0 Comm: swapper Not tainted 4.11.0-next-20170508+ #2
>> [  143.030539] task: c8533580 task.stack: c852c000
>> [  143.035237] EIP: stmmac_xmit+0xf1/0x1080
>> [  143.039302] EFLAGS: 00010216 CPU: 0
>> [  143.042915] EAX: 00000000 EBX: 00000050 ECX: 00000000 EDX: ceb6a0c0
>> [  143.049326] ESI: 00000000 EDI: cdd16000 EBP: cdc25d70 ESP: cdc25d20
>> [  143.055735]  DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
>> [  143.061271] CR0: 80050033 CR2: 00000000 CR3: 0eb5c000 CR4: 00100010
>> [  143.067671] Call Trace:
>> [  143.070238]  <SOFTIRQ>
>> [  143.072763]  dev_hard_start_xmit+0x7c/0x1a0
>> [  143.077120]  sch_direct_xmit+0xf0/0x120
>> [  143.081130]  __dev_queue_xmit+0x181/0x430
>> [  143.085311]  ? eth_commit_mac_addr_change+0x20/0x20
>> [  143.090362]  dev_queue_xmit+0xa/0x10
>> [  143.094100]  neigh_resolve_output+0xdb/0x190
>> [  143.098561]  ip6_finish_output2+0x184/0x500
>> [  143.102945]  ip6_finish_output+0x91/0xe0
>> [  143.107057]  ? ip6_finish_output+0x91/0xe0
>> [  143.111338]  ip6_output+0x36/0x110
>> [  143.114924]  ? ip6_fragment+0xb00/0xb00
>> [  143.118935]  mld_sendpack+0x191/0x2b0
>> [  143.122769]  ? mld_newpack+0xda/0x180
>> [  143.126598]  ? ipv6_icmp_sysctl_init+0x30/0x30
>> [  143.131224]  mld_ifc_timer_expire+0x158/0x240
>> [  143.135756]  ? find_next_bit+0xa/0x10
>> [  143.139584]  ? mld_dad_timer_expire+0x50/0x50
>> [  143.144112]  call_timer_fn+0x2a/0xf0
>> [  143.147862]  ? mld_dad_timer_expire+0x50/0x50
>> [  143.152395]  run_timer_softirq+0x158/0x300
>> [  143.156668]  ? file_free_rcu+0x1e/0x30
>> [  143.160589]  __do_softirq+0xc4/0x200
>> [  143.164341]  ? __hrtimer_tasklet_trampoline+0x30/0x30
>> [  143.169575]  do_softirq_own_stack+0x1e/0x30
>> [  143.173902]  </SOFTIRQ>
>> [  143.176502]  irq_exit+0x95/0xa0
>> [  143.179812]  smp_apic_timer_interrupt+0x31/0x40
>> [  143.184530]  apic_timer_interrupt+0x32/0x40
>> [  143.188889] EIP: default_idle+0xc/0x70
>> [  143.192774] EFLAGS: 00000246 CPU: 0
>> [  143.196386] EAX: 00000000 EBX: 00000000 ECX: 00000001 EDX: 00000000
>> [  143.202795] ESI: 00000000 EDI: c8533580 EBP: c852df54 ESP: c852df4c
>> [  143.209205]  DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
>> [  143.214780]  arch_cpu_idle+0x9/0x10
>> [  143.218446]  default_idle_call+0x17/0x30
>> [  143.222551]  do_idle+0xed/0x130
>> [  143.225873]  cpu_startup_entry+0x15/0x20
>> [  143.229965]  rest_init+0x5c/0x60
>> [  143.233370]  start_kernel+0x313/0x318
>> [  143.237221]  i386_start_kernel+0x98/0x9c
>> [  143.241315]  startup_32_smp+0x16b/0x16d
>> [  143.245289] Code: 84 45 06 00 00 c1 e2 05 03 94 c7 9c 09 00 00 89
>> 55 b0 8b 45 c8 8b 75 bc 8b 55 d8 8d 1c 80 89
>> 75 e4 c1 e3 03 8b 84 1f a4 09 00 00 <89> 14 b0 8b 87 40 0d 00 00 8b 40
>> 24 85 c0 89 45 b8 0f 85 68 02
>> [  143.264746] EIP: stmmac_xmit+0xf1/0x1080 SS:ESP: 0068:cdc25d20
>> [  143.270727] CR2: 0000000000000000
>> [  143.274175] ---[ end trace 79da8ef70f8b98d7 ]---
>> [  143.278925] Kernel panic - not syncing: Fatal exception in interrupt
>> [  143.285433] Kernel Offset: 0x6a00000 from 0xc1000000 (relocation
>> range: 0xc0000000-0xd05effff)
>> [  143.294268] ---[ end Kernel panic - not syncing: Fatal exception in interrupt
>>
>>
>
> Are you using the same version of Ethernet IP, 10/100?

I'm running on Intel Galileo Gen2 board (v4.11 by the way works fine
with direct boot from SD card)

> Could you please verify if the crash you are experiencing is this place?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#n2956
>
> I would say that for rather old IPs, the napi is not capable of giving a valid
> queue number. Could you please print the queue index returned by this line?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#n2948

--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2953,7 +2953,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff
*skb, struct net_device *dev)
       unsigned int enh_desc;
       unsigned int des;

+       pr_info("%s <<< 1: priv %p, queue: %u\n", __func__, priv, queue);
       tx_q = &priv->tx_queue[queue];
+       pr_info("%s <<< 2: priv %p, queue: %u tx_q: %p\n", __func__,
priv, queue, tx_q);


[  101.591040] stmmac_xmit <<< 1: priv cdd1c4c0, queue: 7
[  101.596377] stmmac_xmit <<< 2: priv cdd1c4c0, queue: 7 tx_q: cdd1caac


Also noticed warning that have to be addressed:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2504:49: warning:
incorrect type in argument 1 (different address spaces)
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2504:49:    expected
void [noderef] <asn:2>*ioaddr
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2504:49:    got
struct mac_device_info *hw

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function
‘init_dma_rx_desc_rings’:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1274:15: warning:
comparison of
unsigned expression >= 0 is always true [-Wtype-limits]
 while (queue >= 0) {
              ^~

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v3 net-next 01/11] net: stmmac: prepare dma op mode config for multiple queues
From: Joao Pinto @ 2017-05-08 10:42 UTC (permalink / raw)
  To: Andy Shevchenko, Joao Pinto
  Cc: Jan Kiszka, David S. Miller, Giuseppe CAVALLARO, Alexandre TORGUE,
	netdev, Linux Kernel Mailing List
In-Reply-To: <CAHp75VcO13o0BgZ75Ssxav2jwP_g+WXLjob8x5QTB6Mdoynpzg@mail.gmail.com>

Às 11:12 AM de 5/8/2017, Andy Shevchenko escreveu:
> On Mon, May 8, 2017 at 12:54 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>> Hi Andy and Jan,
>>
>> Às 10:36 AM de 5/8/2017, Andy Shevchenko escreveu:
>>> On Mon, May 8, 2017 at 9:56 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2017-03-15 12:04, Joao Pinto wrote:
>>>>> This patch prepares DMA Operation Mode configuration for multiple queues.
>>>>> The work consisted on breaking the DMA operation Mode configuration function
>>>>> into RX and TX scope and adapting its mechanism in stmmac_main.
>>>
>>>> Starting with this patch, the stmmac-based network adapters of the Intel
>>>> Quark SoC stop working. I'm getting an IP via DHCP, I can ping, but TCP
>>>> connections can no longer be established.
> 
>>> JFYI: With today's linux-next when _kexec:ed_ kernel boots I tried and
>>> got the following:
>>>

snip (...)

>>>
>>>
>>
>> Are you using the same version of Ethernet IP, 10/100?
> 
> I'm running on Intel Galileo Gen2 board (v4.11 by the way works fine
> with direct boot from SD card)
> 
>> Could you please verify if the crash you are experiencing is this place?
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fmain.c-23n2956&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=UF269QZ9ExFRw1XXpgdvO2QeTCLEp-GquRe8OqZwRf0&s=yZu3uME5PK-3nJlxz-H-HfHh3Shjzg0je5If_jSXVb4&e= 
>>
>> I would say that for rather old IPs, the napi is not capable of giving a valid
>> queue number. Could you please print the queue index returned by this line?
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fmain.c-23n2948&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=UF269QZ9ExFRw1XXpgdvO2QeTCLEp-GquRe8OqZwRf0&s=p_TgHODJum23I2N4AldR4oIaOPffSDpk9agmbRMQgoM&e= 
> 
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2953,7 +2953,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff
> *skb, struct net_device *dev)
>        unsigned int enh_desc;
>        unsigned int des;
> 
> +       pr_info("%s <<< 1: priv %p, queue: %u\n", __func__, priv, queue);
>        tx_q = &priv->tx_queue[queue];
> +       pr_info("%s <<< 2: priv %p, queue: %u tx_q: %p\n", __func__,
> priv, queue, tx_q);
> 
> 
> [  101.591040] stmmac_xmit <<< 1: priv cdd1c4c0, queue: 7
> [  101.596377] stmmac_xmit <<< 2: priv cdd1c4c0, queue: 7 tx_q: cdd1caac
> 

I assume that the queue index is always 7 right? By return 7, the napi interface
'thinks' that your setup is using 8 TX queues which I assume it is not and thats
the problem causing your board to malfuntion.

Could you please check the values of the 'real' tx and rx queues count in this line?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#n4107

For default they are =1, so napi should be assuming 1RX and 1TX, and so you
should be getting queue index =0 in reception and transmission.

In terms of reception, could you print the queue index that stmmac_poll is using
here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#n3468

> 
> Also noticed warning that have to be addressed:
> 
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2504:49: warning:
> incorrect type in argument 1 (different address spaces)
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2504:49:    expected
> void [noderef] <asn:2>*ioaddr
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2504:49:    got
> struct mac_device_info *hw

This one was well caught! Although it has no influence in your setup, since you
don't have this callback implemented, eQOS (>= 4.00) and 1000 cores will have
issues if using PCS. I can make a patch for this one.

> 
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function
> ‘init_dma_rx_desc_rings’:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1274:15: warning:
> comparison of
> unsigned expression >= 0 is always true [-Wtype-limits]
>  while (queue >= 0) {
>               ^~

This one I have in my agenda to improve it, I also talked about it with Dan
Carpenter about it.

^ permalink raw reply

* Re: bpf pointer alignment validation
From: Daniel Borkmann @ 2017-05-08 10:49 UTC (permalink / raw)
  To: David Miller, ast; +Cc: netdev
In-Reply-To: <20170505.224709.1156323937148435706.davem@davemloft.net>

On 05/06/2017 04:47 AM, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Fri, 05 May 2017 16:20:44 -0400 (EDT)
>
>> Anyways, I'll play with this design and see what happens...
>> Feedback is of course welcome.
>
> Here is a prototype that works for me with test_pkt_access.c,
> which otherwise won't load on sparc.

Code looks good to me as far as I can tell, thanks for working
on this.

Could you also add test cases specifically to this for test_verifier
in bpf selftests? I'm thinking of the cases when we have no pkt id
and offset originated from reg->off (accumulated through const imm
ops on reg) and insn->off, where we had i) no pkt id and ii) a
specific pkt id (so we can probe for aux_off_align rejection as well).
I believe we do have coverage to some extend in some of the tests
(more on the map_value_adj though), but it would be good to keep
tracking this specifically as well.

Thanks a lot,
Daniel

^ permalink raw reply

* [PATCH net] ip6_tunnel: remove unreachable ICMP_REDIRECT code
From: Hangbin Liu @ 2017-05-08 11:11 UTC (permalink / raw)
  To: netdev; +Cc: Hangbin Liu

After call ip6_tnl_err(), the rel_type will be ether ICMPV6_DEST_UNREACH
or ICMPV6_PKT_TOOBIG. We will never reach ICMP_REDIRECT. So remove it.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/ipv6/ip6_tunnel.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 6eb2ae5..16f8d42 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -591,9 +591,6 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 		rel_type = ICMP_DEST_UNREACH;
 		rel_code = ICMP_FRAG_NEEDED;
 		break;
-	case NDISC_REDIRECT:
-		rel_type = ICMP_REDIRECT;
-		rel_code = ICMP_REDIR_HOST;
 	default:
 		return 0;
 	}
@@ -652,8 +649,6 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 
 		skb_dst(skb2)->ops->update_pmtu(skb_dst(skb2), NULL, skb2, rel_info);
 	}
-	if (rel_type == ICMP_REDIRECT)
-		skb_dst(skb2)->ops->redirect(skb_dst(skb2), NULL, skb2);
 
 	icmp_send(skb2, rel_type, rel_code, htonl(rel_info));
 
-- 
2.5.5

^ permalink raw reply related

* Re: [PATCH/RFC net-next v2 1/4] flow dissector: return error on port dissection under-run
From: Jamal Hadi Salim @ 2017-05-08 11:21 UTC (permalink / raw)
  To: Simon Horman, Jiri Pirko, Cong Wang
  Cc: Dinan Gunawardena, netdev, oss-drivers, Benjamin LaHaise
In-Reply-To: <1493988426-22854-2-git-send-email-simon.horman@netronome.com>

On 17-05-05 08:47 AM, Simon Horman wrote:
> Return an error from __skb_flow_dissect() if insufficient packet data is
> present when dissecting layer 4 ports.
>
> Without this patch the absence of ports in truncated - e.g. UDP - packets
> is treated the same way by the flow dissector as the presence of ports with
> a value of zero. And without this patch the flower classifier is unable to
> differentiate between these two cases which may lead to unexpected matching
> of truncated packets.
>
> With this patch the flow dissector and in turn the flower classifier can
> differentiate between packets with zero L4 ports and truncated packets.
>
> The approach taken here is to only return an error if the offset of ports
> for the previously dissected IP protocol is known - a non error return from
> proto_ports_offset() - but port data is not present in the packet - an
> error return value from __skb_header_pointer().
>
> The behaviour for callers of __skb_flow_get_ports() is changed but the only
> callers are skb_flow_get_ports() and the flow dissector.  The former has
> been updated so that its behaviour is unchanged.  Behavioural change of the
> latter is the intended purpose of this patch but will only take effect with
> a separate patch to have it refuse to match if dissection fails.
>
> This change will lead to behavioural changes of the users of the dissector
> with FLOW_DISSECTOR_KEY_PORTS - flower, and users of
> flow_keys_dissector_keys[] and flow_keys_dissector_symmetric_keys[].  The
> behavioural change for *_keys[] changes seem reasonable as the change will
> should only be for truncated packets.
>
> Signed-off-by: Simon Horman <simon.horman@netronome.com>
> Reviewed-by: Benjamin LaHaise <benjamin.lahaise@netronome.com>

Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

^ permalink raw reply

* Re: [PATCH/RFC net-next v2 2/4] flow dissector: return error on icmp dissection under-run
From: Jamal Hadi Salim @ 2017-05-08 11:21 UTC (permalink / raw)
  To: Simon Horman, Jiri Pirko, Cong Wang
  Cc: Dinan Gunawardena, netdev, oss-drivers, Benjamin LaHaise
In-Reply-To: <1493988426-22854-3-git-send-email-simon.horman@netronome.com>

On 17-05-05 08:47 AM, Simon Horman wrote:
> Return an error from __skb_flow_dissect() if insufficient packet data is
> present when dissecting icmp type and code.
>
> Without this patch the absence of the ICMP type and code in truncated
> ICMPv4 or IPVPv6 packets is treated the way same as the presence of a code
> and type of value of zero.  And without this patch the flower classifier is
> unable to differentiate between these two cases which may lead to
> unexpected matching of truncated packets.
>
> With this patch the flow dissector and in turn the flower classifier can
> differentiate between packets with zero ICMP type and code, and truncated
> packets.
>
> The approach taken here is to return an error if the IP protocol indicates
> ICMP but the type and code data is not present in the packet - an error
> return value from __skb_header_pointer().
>
> This should only effect the flower classifier as it is the only user of
> W_DISSECTOR_KEY_ICMP.  The behavioural update for flower only takes effect
> with a separate patch to have it refuse to match if dissection fails.
>
> Signed-off-by: Simon Horman <simon.horman@netronome.com>
> Reviewed-by: Benjamin LaHaise <benjamin.lahaise@netronome.com>

Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

^ permalink raw reply

* Re: [PATCH/RFC net-next v2 3/4] net/sched: cls_flower: do not match if dissection fails
From: Jamal Hadi Salim @ 2017-05-08 11:26 UTC (permalink / raw)
  To: Simon Horman, Jiri Pirko, Cong Wang
  Cc: Dinan Gunawardena, netdev, oss-drivers, Benjamin LaHaise
In-Reply-To: <1493988426-22854-4-git-send-email-simon.horman@netronome.com>

On 17-05-05 08:47 AM, Simon Horman wrote:
> If the flow skb_flow_dissect() returns an error it indicates that
> dissection was incomplete for some reason. Matching using the result of an
> incomplete dissection may cause unexpected results. For example:
>
> * A match on zero layer 4 ports will also match packets truncated at
>   the end of the IP header; that is packets where ports are missing are
>   treated the same way as packets with zero ports.
> * Likewise, a match on zero ICMP code or type will also match packets
>   truncated at the end of the IP header; that is packets where the ICMP
>   type and code are missing will be treated the same way as packets with
>   zero ICMP code and type.
>
> Separate patches to the flow dissector are required in order for it to
> return errors in the above cases.
>
> Fixes: 77b9900ef53a ("tc: introduce Flower classifier")
> Signed-off-by: Simon Horman <simon.horman@netronome.com>
> Reviewed-by: Benjamin LaHaise <benjamin.lahaise@netronome.com>

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

^ permalink raw reply

* [PATCH] net/fsl: remove func xgmac_wait_until_free() as duplicate
From: Alexandru Ardelean @ 2017-05-08 11:31 UTC (permalink / raw)
  To: netdev; +Cc: Shaohui.Xie, davem, Alexandru Ardelean

Looking at xgmac_wait_until_done() and xgmac_wait_until_free()
functions, they seem to have turned out completely identical.

Though, judging from the git history it seems they
initially weren't.

Remove xgmac_wait_until_free() in favor of xgmac_wait_until_done().

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
---
 drivers/net/ethernet/freescale/xgmac_mdio.c | 33 ++++-------------------------
 1 file changed, 4 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index e03b30c..54597a8 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -71,31 +71,6 @@ static void xgmac_write32(u32 value,
 }
 
 /*
- * Wait until the MDIO bus is free
- */
-static int xgmac_wait_until_free(struct device *dev,
-				 struct tgec_mdio_controller __iomem *regs,
-				 bool is_little_endian)
-{
-	unsigned int timeout;
-
-	/* Wait till the bus is free */
-	timeout = TIMEOUT;
-	while ((xgmac_read32(&regs->mdio_stat, is_little_endian) &
-		MDIO_STAT_BSY) && timeout) {
-		cpu_relax();
-		timeout--;
-	}
-
-	if (!timeout) {
-		dev_err(dev, "timeout waiting for bus to be free\n");
-		return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
-/*
  * Wait till the MDIO read or write operation is complete
  */
 static int xgmac_wait_until_done(struct device *dev,
@@ -147,7 +122,7 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
 
 	xgmac_write32(mdio_stat, &regs->mdio_stat, endian);
 
-	ret = xgmac_wait_until_free(&bus->dev, regs, endian);
+	ret = xgmac_wait_until_done(&bus->dev, regs, endian);
 	if (ret)
 		return ret;
 
@@ -159,7 +134,7 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
 	if (regnum & MII_ADDR_C45) {
 		xgmac_write32(regnum & 0xffff, &regs->mdio_addr, endian);
 
-		ret = xgmac_wait_until_free(&bus->dev, regs, endian);
+		ret = xgmac_wait_until_done(&bus->dev, regs, endian);
 		if (ret)
 			return ret;
 	}
@@ -201,7 +176,7 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
 
 	xgmac_write32(mdio_stat, &regs->mdio_stat, endian);
 
-	ret = xgmac_wait_until_free(&bus->dev, regs, endian);
+	ret = xgmac_wait_until_done(&bus->dev, regs, endian);
 	if (ret)
 		return ret;
 
@@ -213,7 +188,7 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
 	if (regnum & MII_ADDR_C45) {
 		xgmac_write32(regnum & 0xffff, &regs->mdio_addr, endian);
 
-		ret = xgmac_wait_until_free(&bus->dev, regs, endian);
+		ret = xgmac_wait_until_done(&bus->dev, regs, endian);
 		if (ret)
 			return ret;
 	}
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH/RFC net-next v2 4/4] net/sched: cls_flower: allow control of tree traversal on packet parse errors
From: Jamal Hadi Salim @ 2017-05-08 11:32 UTC (permalink / raw)
  To: Simon Horman, Jiri Pirko, Cong Wang
  Cc: Dinan Gunawardena, netdev, oss-drivers, Benjamin LaHaise
In-Reply-To: <1493988426-22854-5-git-send-email-simon.horman@netronome.com>

On 17-05-05 08:47 AM, Simon Horman wrote:
> Allow control how the tree of qdisc, classes and filters is further
> traversed if an error is encountered when parsing the packet in order to
> match the cls_flower filters at a particular prio.
>
> By default continue to the next filter, the behaviour without this patch.
>
> A use-case for this is to allow configuration of dropping of packets with
> truncated headers.
>
> For example, the following drops IPv4 packets that cannot be parsed by the
> flow dissector up to the end of the UDP ports - e.g. because they are
> truncated, and instantiates a continue action based on the port for packets
> that can be parsed.
>
>  # tc qdisc del dev eth0 ingress; tc qdisc add dev eth0 ingress
>  # tc filter add dev eth0 protocol ip parent ffff: flower \
>        indev eth0 ip_proto udp dst_port 80 truncated drop action continue
>
> Signed-off-by: Simon Horman <simon.horman@netronome.com>
> Reviewed-by: Benjamin LaHaise <benjamin.lahaise@netronome.com>

I agree with Cong on this. The default should be "didnt match" (which
is accomplished by returning -1). The user could enter an explicit
rule to override this behavior. i.e something like:

tc filter add dev eth0 protocol ip parent ffff: flower \
         indev eth0 ip_proto udp dst_port 80 truncated action continue

cheers,
jamal

^ permalink raw reply

* Re: [PATCH v3 net-next 01/11] net: stmmac: prepare dma op mode config for multiple queues
From: Andy Shevchenko @ 2017-05-08 11:34 UTC (permalink / raw)
  To: Joao Pinto
  Cc: Jan Kiszka, David S. Miller, Giuseppe CAVALLARO, Alexandre TORGUE,
	netdev, Linux Kernel Mailing List
In-Reply-To: <967da95a-cc4e-a0ac-c702-e659bd4f0481@synopsys.com>

On Mon, May 8, 2017 at 1:42 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
> Às 11:12 AM de 5/8/2017, Andy Shevchenko escreveu:
>> On Mon, May 8, 2017 at 12:54 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>> Às 10:36 AM de 5/8/2017, Andy Shevchenko escreveu:

>>>> JFYI: With today's linux-next when _kexec:ed_ kernel boots I tried and
>>>> got the following:

>>> Are you using the same version of Ethernet IP, 10/100?
>>
>> I'm running on Intel Galileo Gen2 board (v4.11 by the way works fine
>> with direct boot from SD card)
>>
>>> Could you please verify if the crash you are experiencing is this place?
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fmain.c-23n2956&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=UF269QZ9ExFRw1XXpgdvO2QeTCLEp-GquRe8OqZwRf0&s=yZu3uME5PK-3nJlxz-H-HfHh3Shjzg0je5If_jSXVb4&e=
>>>
>>> I would say that for rather old IPs, the napi is not capable of giving a valid
>>> queue number. Could you please print the queue index returned by this line?
>>>
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fmain.c-23n2948&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=UF269QZ9ExFRw1XXpgdvO2QeTCLEp-GquRe8OqZwRf0&s=p_TgHODJum23I2N4AldR4oIaOPffSDpk9agmbRMQgoM&e=
>>
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -2953,7 +2953,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff
>> *skb, struct net_device *dev)
>>        unsigned int enh_desc;
>>        unsigned int des;
>>
>> +       pr_info("%s <<< 1: priv %p, queue: %u\n", __func__, priv, queue);
>>        tx_q = &priv->tx_queue[queue];
>> +       pr_info("%s <<< 2: priv %p, queue: %u tx_q: %p\n", __func__,
>> priv, queue, tx_q);
>>
>>
>> [  101.591040] stmmac_xmit <<< 1: priv cdd1c4c0, queue: 7
>> [  101.596377] stmmac_xmit <<< 2: priv cdd1c4c0, queue: 7 tx_q: cdd1caac

> I assume that the queue index is always 7 right? By return 7, the napi interface
> 'thinks' that your setup is using 8 TX queues which I assume it is not and thats
> the problem causing your board to malfuntion.
>
> Could you please check the values of the 'real' tx and rx queues count in this line?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#n4107
>
> For default they are =1, so napi should be assuming 1RX and 1TX, and so you
> should be getting queue index =0 in reception and transmission.
>
> In terms of reception, could you print the queue index that stmmac_poll is using
> here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#n3468

+       pr_info("%s <<< %u\n", __func__, rx_q->queue_index);
       work_done = stmmac_rx(priv, budget, rx_q->queue_index);
       if (work_done < budget) {
               napi_complete_done(napi, work_done);

       /* Configure real RX and TX queues */
       netif_set_real_num_rx_queues(ndev, priv->plat->rx_queues_to_use);
       netif_set_real_num_tx_queues(ndev, priv->plat->tx_queues_to_use);
+       pr_info("%s <<< %hhu %hhu\n", __func__,
priv->plat->rx_queues_to_use, priv->plat->tx_queues_to_use);


[   44.374161] stmmac_dvr_probe <<< 0 0

[  109.014763] stmmac_xmit <<< 1: priv cdcea4c0, queue: 2
[  109.020099] stmmac_xmit <<< 2: priv cdcea4c0, queue: 2 tx_q: cdcea9e4

That's all, no poll activated.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v3 net-next 01/11] net: stmmac: prepare dma op mode config for multiple queues
From: Joao Pinto @ 2017-05-08 11:40 UTC (permalink / raw)
  To: Andy Shevchenko, Joao Pinto
  Cc: Jan Kiszka, David S. Miller, Giuseppe CAVALLARO, Alexandre TORGUE,
	netdev, Linux Kernel Mailing List
In-Reply-To: <CAHp75Vea1jke4yp0=762B0XvFkcP+YrW=KA7C-v6w61_s4EzUA@mail.gmail.com>

Às 12:34 PM de 5/8/2017, Andy Shevchenko escreveu:
> On Mon, May 8, 2017 at 1:42 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>> Às 11:12 AM de 5/8/2017, Andy Shevchenko escreveu:
>>> On Mon, May 8, 2017 at 12:54 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>>> Às 10:36 AM de 5/8/2017, Andy Shevchenko escreveu:
> 
>>>>> JFYI: With today's linux-next when _kexec:ed_ kernel boots I tried and
>>>>> got the following:
> 
>>>> Are you using the same version of Ethernet IP, 10/100?
>>>
>>> I'm running on Intel Galileo Gen2 board (v4.11 by the way works fine
>>> with direct boot from SD card)
>>>
>>>> Could you please verify if the crash you are experiencing is this place?
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fmain.c-23n2956&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=UF269QZ9ExFRw1XXpgdvO2QeTCLEp-GquRe8OqZwRf0&s=yZu3uME5PK-3nJlxz-H-HfHh3Shjzg0je5If_jSXVb4&e=
>>>>
>>>> I would say that for rather old IPs, the napi is not capable of giving a valid
>>>> queue number. Could you please print the queue index returned by this line?
>>>>
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fmain.c-23n2948&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=UF269QZ9ExFRw1XXpgdvO2QeTCLEp-GquRe8OqZwRf0&s=p_TgHODJum23I2N4AldR4oIaOPffSDpk9agmbRMQgoM&e=
>>>
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> @@ -2953,7 +2953,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff
>>> *skb, struct net_device *dev)
>>>        unsigned int enh_desc;
>>>        unsigned int des;
>>>
>>> +       pr_info("%s <<< 1: priv %p, queue: %u\n", __func__, priv, queue);
>>>        tx_q = &priv->tx_queue[queue];
>>> +       pr_info("%s <<< 2: priv %p, queue: %u tx_q: %p\n", __func__,
>>> priv, queue, tx_q);
>>>
>>>
>>> [  101.591040] stmmac_xmit <<< 1: priv cdd1c4c0, queue: 7
>>> [  101.596377] stmmac_xmit <<< 2: priv cdd1c4c0, queue: 7 tx_q: cdd1caac
> 
>> I assume that the queue index is always 7 right? By return 7, the napi interface
>> 'thinks' that your setup is using 8 TX queues which I assume it is not and thats
>> the problem causing your board to malfuntion.
>>
>> Could you please check the values of the 'real' tx and rx queues count in this line?
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fmain.c-23n4107&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=6PN46fgWi1XTHkxFzV9wkYHPkKJWvkRC1OOlEhyKdcA&s=cyYmWeYuPwacYmVRzJbhRm3Krz6XNyHbxq8t7ZUi8Ec&e= 
>>
>> For default they are =1, so napi should be assuming 1RX and 1TX, and so you
>> should be getting queue index =0 in reception and transmission.
>>
>> In terms of reception, could you print the queue index that stmmac_poll is using
>> here:
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fmain.c-23n3468&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=6PN46fgWi1XTHkxFzV9wkYHPkKJWvkRC1OOlEhyKdcA&s=Xli0e7Key3FA7Rve_opcwc6W7nd4khVX15wwoNpFHL4&e= 
> 
> +       pr_info("%s <<< %u\n", __func__, rx_q->queue_index);
>        work_done = stmmac_rx(priv, budget, rx_q->queue_index);
>        if (work_done < budget) {
>                napi_complete_done(napi, work_done);
> 
>        /* Configure real RX and TX queues */
>        netif_set_real_num_rx_queues(ndev, priv->plat->rx_queues_to_use);
>        netif_set_real_num_tx_queues(ndev, priv->plat->tx_queues_to_use);
> +       pr_info("%s <<< %hhu %hhu\n", __func__,
> priv->plat->rx_queues_to_use, priv->plat->tx_queues_to_use);
> 
> 
> [   44.374161] stmmac_dvr_probe <<< 0 0
> 

Ok, so this is the cause of the problem. The driver is geting 0 for real RX and
TX queues.

Your setup uses standard DT parsing from stmmac_platform or a custom one?

If you are using stmmac_probe_config_dt():
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c#n363

You will find a function named stmmac_mtl_setup() being called:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c#n492

In this function, the number of RX and TX queues is being set to 1 by default.

Joao


> [  109.014763] stmmac_xmit <<< 1: priv cdcea4c0, queue: 2
> [  109.020099] stmmac_xmit <<< 2: priv cdcea4c0, queue: 2 tx_q: cdcea9e4
> 
> That's all, no poll activated.
> 

^ permalink raw reply

* Re: net/key: slab-out-of-bounds in pfkey_compile_policy
From: Steffen Klassert @ 2017-05-08 11:49 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Herbert Xu, David S. Miller, netdev, LKML, Dmitry Vyukov,
	Kostya Serebryany, Eric Dumazet, Cong Wang, syzkaller
In-Reply-To: <CAAeHK+xjbBPqHgBwBEK8=p7zUNCA144GqDSJMwUvz-1NFNQWxw@mail.gmail.com>

On Fri, May 05, 2017 at 02:18:01PM +0200, Andrey Konovalov wrote:
> On Fri, May 5, 2017 at 11:11 AM, Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> > On Tue, May 02, 2017 at 06:45:03PM +0200, Andrey Konovalov wrote:
> >> Hi,
> >>
> >> I've got the following error report while fuzzing the kernel with syzkaller.
> >>
> >> On commit d3b5d35290d729a2518af00feca867385a1b08fa (4.11).
> >>
> >> A reproducer and .config are attached.
> >>
> >> ==================================================================
> >> BUG: KASAN: slab-out-of-bounds in pfkey_compile_policy+0x8e6/0xd40 at
> >> addr ffff88006701f798
> >> Read of size 1280 by task a.out/4181
> >
> >
> > This bug was introduced twelve years ago...
> >
> > This patch is based just on code review, I don't have an option to
> > function test this. But I see that we now exit with -EINVAL before the
> > memcpy that causes the slab-out-of-bounds when using your reproducer,
> > so it should at least fix the bug.
> 
> Hi Steffen,
> 
> This patch fixes the issue for me.
> 
> Thanks!
> 
> Tested-by: Andrey Konovalov <andreyknvl@google.com>

Patch is now applied to the ipsec tree.
Thanks for reporting and testing!

^ permalink raw reply

* Re: [PATCH net v2 1/1] xfrm: Fix NETDEV_DOWN with IPSec offload
From: Steffen Klassert @ 2017-05-08 11:50 UTC (permalink / raw)
  To: ilant; +Cc: David Miller, Boris Pismenny, netdev
In-Reply-To: <20170508073934.28529-2-ilant@mellanox.com>

On Mon, May 08, 2017 at 10:39:34AM +0300, ilant@mellanox.com wrote:
> From: Ilan Tayari <ilant@mellanox.com>
> 
> Upon NETDEV_DOWN event, all xfrm_state objects which are bound to
> the device are flushed.
> 
> The condition for this is wrong, though, testing dev->hw_features
> instead of dev->features. If a device has non-user-modifiable
> NETIF_F_HW_ESP, then its xfrm_state objects are not flushed,
> causing a crash later on after the device is deleted.
> 
> Check dev->features instead of dev->hw_features.
> 
> Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
> Signed-off-by: Ilan Tayari <ilant@mellanox.com>

Applied, thanks Ilan!

^ permalink raw reply

* Re: [PATCH/RFC net-next v2 4/4] net/sched: cls_flower: allow control of tree traversal on packet parse errors
From: Simon Horman @ 2017-05-08 11:54 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: Jiri Pirko, Cong Wang, Dinan Gunawardena, netdev, oss-drivers,
	Benjamin LaHaise
In-Reply-To: <6c618cf6-99ec-d2e1-f1ed-dfae0bf54de7@mojatatu.com>

On Mon, May 08, 2017 at 07:32:02AM -0400, Jamal Hadi Salim wrote:
> On 17-05-05 08:47 AM, Simon Horman wrote:
> >Allow control how the tree of qdisc, classes and filters is further
> >traversed if an error is encountered when parsing the packet in order to
> >match the cls_flower filters at a particular prio.
> >
> >By default continue to the next filter, the behaviour without this patch.
> >
> >A use-case for this is to allow configuration of dropping of packets with
> >truncated headers.
> >
> >For example, the following drops IPv4 packets that cannot be parsed by the
> >flow dissector up to the end of the UDP ports - e.g. because they are
> >truncated, and instantiates a continue action based on the port for packets
> >that can be parsed.
> >
> > # tc qdisc del dev eth0 ingress; tc qdisc add dev eth0 ingress
> > # tc filter add dev eth0 protocol ip parent ffff: flower \
> >       indev eth0 ip_proto udp dst_port 80 truncated drop action continue
> >
> >Signed-off-by: Simon Horman <simon.horman@netronome.com>
> >Reviewed-by: Benjamin LaHaise <benjamin.lahaise@netronome.com>
> 
> I agree with Cong on this. The default should be "didnt match" (which
> is accomplished by returning -1).

The default value for err_action is TC_ACT_UNSPEC (-1).
So I think we are in agreement there.

> The user could enter an explicit
> rule to override this behavior. i.e something like:
> 
> tc filter add dev eth0 protocol ip parent ffff: flower \
>         indev eth0 ip_proto udp dst_port 80 truncated action continue

This part I am struggling with but I will see what I can do
by making truncated part of the flow key .

^ permalink raw reply

* Re: [PATCH v3 net-next 01/11] net: stmmac: prepare dma op mode config for multiple queues
From: Andy Shevchenko @ 2017-05-08 11:56 UTC (permalink / raw)
  To: Joao Pinto
  Cc: Jan Kiszka, David S. Miller, Giuseppe CAVALLARO, Alexandre TORGUE,
	netdev, Linux Kernel Mailing List
In-Reply-To: <c80563da-d4f7-5281-d692-d05f43fc80b6@synopsys.com>

On Mon, May 8, 2017 at 2:40 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
> Às 12:34 PM de 5/8/2017, Andy Shevchenko escreveu:
>> On Mon, May 8, 2017 at 1:42 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>> Às 11:12 AM de 5/8/2017, Andy Shevchenko escreveu:
>>>> On Mon, May 8, 2017 at 12:54 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>>>> Às 10:36 AM de 5/8/2017, Andy Shevchenko escreveu:

>>
>> [   44.374161] stmmac_dvr_probe <<< 0 0
>>
>
> Ok, so this is the cause of the problem. The driver is geting 0 for real RX and
> TX queues.
>
> Your setup uses standard DT parsing from stmmac_platform or a custom one?
>
> If you are using stmmac_probe_config_dt():
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c#n363
>
> You will find a function named stmmac_mtl_setup() being called:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c#n492
>
> In this function, the number of RX and TX queues is being set to 1 by default.

Ah-ha, now I know how it's happened.
You forget to update all setup() hooks in PCI bus driver :-)

I will prepare a fix.
Just tell me should I put Fixes tag or not? And if yes, what commit
should I refer to?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v3 net-next 01/11] net: stmmac: prepare dma op mode config for multiple queues
From: Joao Pinto @ 2017-05-08 12:02 UTC (permalink / raw)
  To: Andy Shevchenko, Joao Pinto
  Cc: Jan Kiszka, David S. Miller, Giuseppe CAVALLARO, Alexandre TORGUE,
	netdev, Linux Kernel Mailing List
In-Reply-To: <CAHp75Vc1YeD9DvwVLh7U1CtDWRGpm-zQSbf4d8C=skLcf_ucBg@mail.gmail.com>

Às 12:56 PM de 5/8/2017, Andy Shevchenko escreveu:
> On Mon, May 8, 2017 at 2:40 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>> Às 12:34 PM de 5/8/2017, Andy Shevchenko escreveu:
>>> On Mon, May 8, 2017 at 1:42 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>>> Às 11:12 AM de 5/8/2017, Andy Shevchenko escreveu:
>>>>> On Mon, May 8, 2017 at 12:54 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>>>>> Às 10:36 AM de 5/8/2017, Andy Shevchenko escreveu:
> 
>>>
>>> [   44.374161] stmmac_dvr_probe <<< 0 0
>>>
>>
>> Ok, so this is the cause of the problem. The driver is geting 0 for real RX and
>> TX queues.
>>
>> Your setup uses standard DT parsing from stmmac_platform or a custom one?
>>
>> If you are using stmmac_probe_config_dt():
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n363&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=fJQj7RiT2sksJYOAZ9VSJUDnxPR7RlE6Fw_cTV0_Mqc&s=KhdAPUtP0twDkibE89cLYs8JjnxEvBgav5uf08WL_e8&e= 
>>
>> You will find a function named stmmac_mtl_setup() being called:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n492&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=fJQj7RiT2sksJYOAZ9VSJUDnxPR7RlE6Fw_cTV0_Mqc&s=rTxn0fwdudwq9XAquH60xNHN538KBQ6_n4wODdLoyA0&e= 
>>
>> In this function, the number of RX and TX queues is being set to 1 by default.
> 
> Ah-ha, now I know how it's happened.
> You forget to update all setup() hooks in PCI bus driver :-)
> 
> I will prepare a fix.
> Just tell me should I put Fixes tag or not? And if yes, what commit
> should I refer to?
> 

Great, you can use this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c?id=26d6851fd24ed5d88580d66b4c8384947d5ca29b

Thanks!

Joao

^ permalink raw reply

* Re: [ISSUE: sky2 - rx error] Link stops working under heavy traffic load connected to a mv88e6176
From: Rafa Corvillo @ 2017-05-08 12:03 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Stephen Hemminger, netdev
In-Reply-To: <20170428122259.GH13231@lunn.ch>

On 28/04/17 14:22, Andrew Lunn wrote:
>>> Since you are using DSA, you will have DSA tags enabled on frames
>>> to/from the switch. This adds an extra 8 byte header in the frame.  My
>>> guess is, it is this header, not the VLAN tag which is causing you MTU
>>> issues.
>>
>> But it is strange because, as I have said above, we have the same
>> configuration working properly on a kernel 4.1 (with OpenWrt), and
>> we have the MTU set to 1500.

Hi Andrew,

Sorry for the delay in my answer, I was out of the office.

>
> If you look at sky2.c:
>
> static unsigned sky2_get_rx_threshold(struct sky2_port *sky2)
> {
>          unsigned size;
>
>          /* Space needed for frame data + headers rounded up */
>          size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);
>
>          /* Stopping point for hardware truncation */
>          return (size - 8) / sizeof(u32);
> }
>
> This is not going to be big enough for a frame with a DSA header.
>

Then, would be a good fix add 8 bytes to the size variable in this function?

>>> I think this is the first time i've seen sky2 used in a DSA
>>> setup. mv643xx or mvneta is generally what is used, when using Marvell
>>> chipsets. These drivers are more lenient about MTU, and are happy to
>>> pass frames with additional headers.
>>>
>>
>> We use the mv88e6xxx (as our switch is mv88e6176) and it depends on
>> DSA driver in the kernel (isn't it?).
>
> That is correct. But i was talking about the Ethernet interface. All
> the designs i've seen use an mv643xxx Ethernet interface, or an mvneta
> interface. This is the first time i've seen a sky2 used, which is why
> i'm not too surprised you have issues.
>
>>> Changing the MTU like this is not a good fix. It will allow you to
>>> receive frames which are bigger, but it also means the local network
>>> stack will generate bigger frames to be transmitted. You probably need
>>> to modify the sky2 driver to allow it to receive frames bigger than
>>> the interface MTU, by about 8 bytes.
>>
>> Should the DSA driver remove the DSA tags before pass the frames to
>> sky2 interface?
>
> The DSA driver is adding the DSA tags to the frame and passing these
> tagged frames to the sky2 interface. Frames going to/from the switch
> will always have such tags.
>
>>>> [ 4901.032989] sky2 0000:04:00.0 marvell: tx timeout
>>>> [ 4904.722670] sky2 0000:04:00.0 marvell: Link is up at 1000 Mbps,
>>>> full duplex, flow control both
>>>
>>> Between the sky2 and the switch, do you have two back-to-back PHYs or
>>> are you connecting the RGMII interfaces together?
>>
>> I think that we have two back-to-back PHYs, but I am going to double
>> check this with the hardware team.
>
> This could be your problem them. The mv88e6xxx switch driver assumes
> there is a straight rgmii-rgmii connection, no PHYs. So it hard
> configures the 'CPU' port to its fastest speed, with the link forced
> up. If you actually have a PHY there, this might not work so well. I
> don't know if the switch PHY is going to do autoneg correctly. Try
> using ethtool to look at the sky2 PHY and see what state it is in.
>
>        Andrew
>

The output of ethtool of sky2 interface is the following:

Settings for marvell:
         Supported ports: [ TP ]
         Supported link modes:   10baseT/Half 10baseT/Full
                                 100baseT/Half 100baseT/Full
                                 1000baseT/Half 1000baseT/Full
         Supported pause frame use: No
         Supports auto-negotiation: Yes
         Advertised link modes:  10baseT/Half 10baseT/Full
                                 100baseT/Half 100baseT/Full
                                 1000baseT/Half 1000baseT/Full
         Advertised pause frame use: No
         Advertised auto-negotiation: No
         Speed: 1000Mb/s
         Duplex: Full
         Port: Twisted Pair
         PHYAD: 0
         Transceiver: internal
         Auto-negotiation: on
         MDI-X: Unknown
         Supports Wake-on: pg
         Wake-on: d
         Current message level: 0x000000ff (255)
                                drv probe link timer ifdown ifup rx_err 
tx_err
         Link detected: yes


And the output of ethtool of eth2@marvell (interface that I have connected):

Settings for eth2:
         Supported ports: [ TP MII ]
         Supported link modes:   10baseT/Half 10baseT/Full
                                 100baseT/Half 100baseT/Full
                                 1000baseT/Half 1000baseT/Full
         Supported pause frame use: No
         Supports auto-negotiation: Yes
         Advertised link modes:  10baseT/Half 10baseT/Full
                                 100baseT/Half 100baseT/Full
                                 1000baseT/Half 1000baseT/Full
         Advertised pause frame use: No
         Advertised auto-negotiation: Yes
         Link partner advertised link modes:  10baseT/Half 10baseT/Full
                                              100baseT/Half 100baseT/Full
         Link partner advertised pause frame use: No
         Link partner advertised auto-negotiation: No
         Speed: 100Mb/s
         Duplex: Full
         Port: MII
         PHYAD: 2
         Transceiver: external
         Auto-negotiation: on
         Supports Wake-on: d
         Wake-on: d
         Link detected: yes


Do you see something strange in these outputs?

Thanks,

Rafa

^ permalink raw reply

* RE: [PATCH] net: dsa: loop: Check for memory allocation failure
From: David Laight @ 2017-05-08 12:05 UTC (permalink / raw)
  To: 'Christophe JAILLET', andrew@lunn.ch,
	vivien.didelot@savoirfairelinux.com, f.fainelli@gmail.com
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org
In-Reply-To: <20170506052945.2639-1-christophe.jaillet@wanadoo.fr>

From: Christophe JAILLET
> Sent: 06 May 2017 06:30
> If 'devm_kzalloc' fails, a NULL pointer will be dereferenced.
> Return -ENOMEM instead, as done for some other memory allocation just a
> few lines above.
...
> --- a/drivers/net/dsa/dsa_loop.c
> +++ b/drivers/net/dsa/dsa_loop.c
> @@ -256,6 +256,9 @@ static int dsa_loop_drv_probe(struct mdio_device *mdiodev)
>  		return -ENOMEM;
> 
>  	ps = devm_kzalloc(&mdiodev->dev, sizeof(*ps), GFP_KERNEL);
> +	if (!ps)
> +		return -ENOMEM;
> +
>  	ps->netdev = dev_get_by_name(&init_net, pdata->netdev);
>  	if (!ps->netdev)
>  		return -EPROBE_DEFER;

On the face if it this code leaks like a sieve.

	David

^ permalink raw reply

* RE: [PATCH] net: dsa: loop: Check for memory allocation failure
From: Julia Lawall @ 2017-05-08 12:32 UTC (permalink / raw)
  To: David Laight
  Cc: 'Christophe JAILLET', andrew@lunn.ch,
	vivien.didelot@savoirfairelinux.com, f.fainelli@gmail.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DCFFE715E@AcuExch.aculab.com>



On Mon, 8 May 2017, David Laight wrote:

> From: Christophe JAILLET
> > Sent: 06 May 2017 06:30
> > If 'devm_kzalloc' fails, a NULL pointer will be dereferenced.
> > Return -ENOMEM instead, as done for some other memory allocation just a
> > few lines above.
> ...
> > --- a/drivers/net/dsa/dsa_loop.c
> > +++ b/drivers/net/dsa/dsa_loop.c
> > @@ -256,6 +256,9 @@ static int dsa_loop_drv_probe(struct mdio_device *mdiodev)
> >  		return -ENOMEM;
> >
> >  	ps = devm_kzalloc(&mdiodev->dev, sizeof(*ps), GFP_KERNEL);
> > +	if (!ps)
> > +		return -ENOMEM;
> > +
> >  	ps->netdev = dev_get_by_name(&init_net, pdata->netdev);
> >  	if (!ps->netdev)
> >  		return -EPROBE_DEFER;
>
> On the face if it this code leaks like a sieve.

I don't think so.  The allocations (dsa_switch_alloc and devm_kzalloc) use
devm functions.

julia

^ permalink raw reply

* Re: [PATCH v3 net-next 01/11] net: stmmac: prepare dma op mode config for multiple queues
From: Jan Kiszka @ 2017-05-08 12:36 UTC (permalink / raw)
  To: Joao Pinto, Andy Shevchenko
  Cc: David S. Miller, Giuseppe CAVALLARO, Alexandre TORGUE, netdev,
	Linux Kernel Mailing List
In-Reply-To: <54cd5a46-eed3-bce7-5280-c5e95957e8e6@synopsys.com>

On 2017-05-08 14:02, Joao Pinto wrote:
> Às 12:56 PM de 5/8/2017, Andy Shevchenko escreveu:
>> On Mon, May 8, 2017 at 2:40 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>> Às 12:34 PM de 5/8/2017, Andy Shevchenko escreveu:
>>>> On Mon, May 8, 2017 at 1:42 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>>>> Às 11:12 AM de 5/8/2017, Andy Shevchenko escreveu:
>>>>>> On Mon, May 8, 2017 at 12:54 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>>>>>> Às 10:36 AM de 5/8/2017, Andy Shevchenko escreveu:
>>
>>>>
>>>> [   44.374161] stmmac_dvr_probe <<< 0 0
>>>>
>>>
>>> Ok, so this is the cause of the problem. The driver is geting 0 for real RX and
>>> TX queues.
>>>
>>> Your setup uses standard DT parsing from stmmac_platform or a custom one?
>>>
>>> If you are using stmmac_probe_config_dt():
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n363&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=fJQj7RiT2sksJYOAZ9VSJUDnxPR7RlE6Fw_cTV0_Mqc&s=KhdAPUtP0twDkibE89cLYs8JjnxEvBgav5uf08WL_e8&e= 
>>>
>>> You will find a function named stmmac_mtl_setup() being called:
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n492&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=fJQj7RiT2sksJYOAZ9VSJUDnxPR7RlE6Fw_cTV0_Mqc&s=rTxn0fwdudwq9XAquH60xNHN538KBQ6_n4wODdLoyA0&e= 
>>>
>>> In this function, the number of RX and TX queues is being set to 1 by default.
>>
>> Ah-ha, now I know how it's happened.
>> You forget to update all setup() hooks in PCI bus driver :-)
>>
>> I will prepare a fix.
>> Just tell me should I put Fixes tag or not? And if yes, what commit
>> should I refer to?
>>
> 
> Great, you can use this commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c?id=26d6851fd24ed5d88580d66b4c8384947d5ca29b
> 
> Thanks!
> 
> Joao
> 

Perfect, looking forward to try out a fix. Thanks, folks!

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply


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