Re: [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()

2015-11-20 Thread Yasushi SHOJI
Hi Sergei,

On Fri, 20 Nov 2015 02:53:39 +0900,
Sergei Shtylyov wrote:
> 
>Shoji-san, can I push this patch to net.git? I doubt that it has
> ill effects in itself -- the reason of the slowdown you're seeing
> should be somewhere else...

Sure.  I've tested and the null access problem is gone for sure.  I'm
pretty sure that the fix won't break anything.

It's going to take, however, some more time to pin down the slow down
problem.  I'll report when I find the cause.

Thanks,
-- 
yashi
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()

2015-11-19 Thread Sergei Shtylyov

Hello.

On 10/27/2015 01:52 AM, Yasushi SHOJI wrote:


In a low memory situation the following kernel oops occurs:

Unable to handle kernel NULL pointer dereference at virtual address 0050
pgd = 8490c000
[0050] *pgd=4651e831, *pte=, *ppte=
Internal error: Oops: 17 [#1] PREEMPT ARM
Modules linked in:
CPU: 0Not tainted  (3.4-at16 #9)
PC is at skb_put+0x10/0x98
LR is at sh_eth_poll+0x2c8/0xa10
pc : [<8035f780>]lr : [<8028bf50>]psr: 6113
sp : 84eb1a90  ip : 84eb1ac8  fp : 84eb1ac4
r10: 003f  r9 : 05ea  r8 : 
r7 :   r6 : 940453b0  r5 : 0003  r4 : 9381b180
r3 :   r2 :   r1 : 05ea  r0 : 
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 10c53c7d  Table: 4248c059  DAC: 0015
Process klogd (pid: 2046, stack limit = 0x84eb02e8)
[...]

This is because netdev_alloc_skb() fails and 'mdp->rx_skbuff[entry]' is left
NULL but sh_eth_rx() later uses it without checking. Add such check...

Reported-by: Yasushi SHOJI 
Signed-off-by: Sergei Shtylyov 

---
This patch is against DaveM's 'net.git' repo.

  drivers/net/ethernet/renesas/sh_eth.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

Index: net/drivers/net/ethernet/renesas/sh_eth.c
===
--- net.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net/drivers/net/ethernet/renesas/sh_eth.c
@@ -1481,6 +1481,7 @@ static int sh_eth_rx(struct net_device *
if (mdp->cd->shift_rd0)
desc_status >>= 16;

+   skb = mdp->rx_skbuff[entry];
if (desc_status & (RD_RFS1 | RD_RFS2 | RD_RFS3 | RD_RFS4 |
   RD_RFS5 | RD_RFS6 | RD_RFS10)) {
ndev->stats.rx_errors++;
@@ -1496,12 +1497,11 @@ static int sh_eth_rx(struct net_device *
ndev->stats.rx_missed_errors++;
if (desc_status & RD_RFS10)
ndev->stats.rx_over_errors++;
-   } else {
+   } else  if (skb) {
if (!mdp->cd->hw_swap)
sh_eth_soft_swap(
phys_to_virt(ALIGN(rxdesc->addr, 4)),
pkt_len + 2);
-   skb = mdp->rx_skbuff[entry];
mdp->rx_skbuff[entry] = NULL;
if (mdp->cd->rpadir)
skb_reserve(skb, NET_IP_ALIGN);



This certainly prevents from a bad access, however, some odd thing is
going on.  Once I hit a low memory situation with this patch, network
thorough-put and response is very bad.



telnet, ping, wget takes loong time.

PING 172.16.2.13 (172.16.2.13) 56(84) bytes of data.
64 bytes from 172.16.2.13: icmp_seq=5 ttl=64 time=0.223 ms
64 bytes from 172.16.2.13: icmp_seq=6 ttl=64 time=0.195 ms
64 bytes from 172.16.2.13: icmp_seq=7 ttl=64 time=0.203 ms
64 bytes from 172.16.2.13: icmp_seq=8 ttl=64 time=0.219 ms
64 bytes from 172.16.2.13: icmp_seq=9 ttl=64 time=0.165 ms
64 bytes from 172.16.2.13: icmp_seq=10 ttl=64 time=0.171 ms
64 bytes from 172.16.2.13: icmp_seq=1 ttl=64 time=9023 ms
64 bytes from 172.16.2.13: icmp_seq=2 ttl=64 time=8022 ms
64 bytes from 172.16.2.13: icmp_seq=3 ttl=64 time=7014 ms
64 bytes from 172.16.2.13: icmp_seq=4 ttl=64 time=6006 ms

I'll investigate it.


   Shoji-san, can I push this patch to net.git? I doubt that it has ill 
effects in itself -- the reason of the slowdown you're seeing should be 
somewhere else...


MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()

2015-10-30 Thread David Miller
From: Sergei Shtylyov 
Date: Sun, 25 Oct 2015 01:42:33 +0300

> In a low memory situation the following kernel oops occurs:
 ...
> This is because netdev_alloc_skb() fails and 'mdp->rx_skbuff[entry]' is left
> NULL but sh_eth_rx() later uses it without checking. Add such check...
> 
> Reported-by: Yasushi SHOJI 
> Signed-off-by: Sergei Shtylyov 

This seems to need more investigation and work, therefore I am
not applying this at this time.

When a final version is available (or even if this one is deemed
appropriate as-is) please resubmit it withut RFT in the Subject.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()

2015-10-26 Thread Yasushi SHOJI
Hi Sergei,

Thank you for your patch!

On Sun, 25 Oct 2015 07:42:33 +0900,
Sergei Shtylyov wrote:
> 
> In a low memory situation the following kernel oops occurs:
> 
> Unable to handle kernel NULL pointer dereference at virtual address 0050
> pgd = 8490c000
> [0050] *pgd=4651e831, *pte=, *ppte=
> Internal error: Oops: 17 [#1] PREEMPT ARM
> Modules linked in:
> CPU: 0Not tainted  (3.4-at16 #9)
> PC is at skb_put+0x10/0x98
> LR is at sh_eth_poll+0x2c8/0xa10
> pc : [<8035f780>]lr : [<8028bf50>]psr: 6113
> sp : 84eb1a90  ip : 84eb1ac8  fp : 84eb1ac4
> r10: 003f  r9 : 05ea  r8 : 
> r7 :   r6 : 940453b0  r5 : 0003  r4 : 9381b180
> r3 :   r2 :   r1 : 05ea  r0 : 
> Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
> Control: 10c53c7d  Table: 4248c059  DAC: 0015
> Process klogd (pid: 2046, stack limit = 0x84eb02e8)
> [...]
> 
> This is because netdev_alloc_skb() fails and 'mdp->rx_skbuff[entry]' is left
> NULL but sh_eth_rx() later uses it without checking. Add such check...
> 
> Reported-by: Yasushi SHOJI 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against DaveM's 'net.git' repo.
> 
>  drivers/net/ethernet/renesas/sh_eth.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Index: net/drivers/net/ethernet/renesas/sh_eth.c
> ===
> --- net.orig/drivers/net/ethernet/renesas/sh_eth.c
> +++ net/drivers/net/ethernet/renesas/sh_eth.c
> @@ -1481,6 +1481,7 @@ static int sh_eth_rx(struct net_device *
>   if (mdp->cd->shift_rd0)
>   desc_status >>= 16;
>  
> + skb = mdp->rx_skbuff[entry];
>   if (desc_status & (RD_RFS1 | RD_RFS2 | RD_RFS3 | RD_RFS4 |
>  RD_RFS5 | RD_RFS6 | RD_RFS10)) {
>   ndev->stats.rx_errors++;
> @@ -1496,12 +1497,11 @@ static int sh_eth_rx(struct net_device *
>   ndev->stats.rx_missed_errors++;
>   if (desc_status & RD_RFS10)
>   ndev->stats.rx_over_errors++;
> - } else {
> + } else  if (skb) {
>   if (!mdp->cd->hw_swap)
>   sh_eth_soft_swap(
>   phys_to_virt(ALIGN(rxdesc->addr, 4)),
>   pkt_len + 2);
> - skb = mdp->rx_skbuff[entry];
>   mdp->rx_skbuff[entry] = NULL;
>   if (mdp->cd->rpadir)
>   skb_reserve(skb, NET_IP_ALIGN);
> 

This certainly prevents from a bad access, however, some odd thing is
going on.  Once I hit a low memory situation with this patch, network
thorough-put and response is very bad.

telnet, ping, wget takes loong time.

PING 172.16.2.13 (172.16.2.13) 56(84) bytes of data.
64 bytes from 172.16.2.13: icmp_seq=5 ttl=64 time=0.223 ms
64 bytes from 172.16.2.13: icmp_seq=6 ttl=64 time=0.195 ms
64 bytes from 172.16.2.13: icmp_seq=7 ttl=64 time=0.203 ms
64 bytes from 172.16.2.13: icmp_seq=8 ttl=64 time=0.219 ms
64 bytes from 172.16.2.13: icmp_seq=9 ttl=64 time=0.165 ms
64 bytes from 172.16.2.13: icmp_seq=10 ttl=64 time=0.171 ms
64 bytes from 172.16.2.13: icmp_seq=1 ttl=64 time=9023 ms
64 bytes from 172.16.2.13: icmp_seq=2 ttl=64 time=8022 ms
64 bytes from 172.16.2.13: icmp_seq=3 ttl=64 time=7014 ms
64 bytes from 172.16.2.13: icmp_seq=4 ttl=64 time=6006 ms

I'll investigate it.
-- 
  yashi
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html