Re: CVS commit: src/sys/dev/pci/ixgbe

2018-05-07 Thread Masanobu SAITOH

On 2018/05/07 16:56, Masanobu SAITOH wrote:

Hi.

On 2018/05/03 17:38, Frank Kardel wrote:

Hi,

I am now seeing that packet reception stops. tcpdump will only show outgoing 
packets and no incoming packets.

ifconfig ixgX down
ifconfig ixgX up

gets things going again.

Code from 2018-02-24 was fine and I think even code shortly before this commit 
was fine, but there was the mbuf used-after-free issue that hit me pretty hard. 
So the system wasn't running for too long anyway.

Does that ring a bell ?

Frank


  IMHO, this change doesn't cause such type of problem...

  Could you show me the following information?

  0) dmesg

 This is required to know which chip is used and what type of
 interrupt is used and configured (INTX, MSI or MSI-X).

  1) ifconfig -v ixg0

 It's required to know offload status, the MTU size and some
 error count.

  2) sysctl hw.ixg0 (or sysctl hw |grep ixg (for all ixg(4)s))

 This output has some information of TX/RX ring.

  3) vmstat -e[v]

     Only for ixg(4):
     vmstat -e[v] |grep ixg

 ixg(4) has a lot of counters. I added error counters at
 all of error paths. (If you found a error path which has
 no counter, it's my fault.)

     Only to check checksum counters:

     options INET_CSUM_COUNTERS (for ip_input())
     options TCP_CSUM_COUNTERS  (for tcp_input())
     options UDP_CSUM_COUNTERS  (for udp_input())
     options WM_EVENT_COUNTERS  (only for wm(4))
     options BGE_EVENT_COUNTERS (only for bge(4))
     (No special options for ixg(4) because those counters are enabled by 
default)

     vmstat -e[v] |grep csum

  4) If you use a virtual interface like vlan(4) or age(4). Please


s/age/agr/



     show the configuration.



5) Whether you use options NET_MPSAFE or not.


Thanks.



  Regards.


On 04/25/18 10:46, SAITOH Masanobu wrote:

Module Name:    src
Committed By:    msaitoh
Date:    Wed Apr 25 08:46:19 UTC 2018

Modified Files:
src/sys/dev/pci/ixgbe: ix_txrx.c ixgbe.h ixgbe_netbsd.c ixgbe_netbsd.h
    ixgbe_osdep.h

Log Message:
  Don't free and reallocate bus_dmamem when it's not required. Currently,
the watchdog timer is completely broken and never fire (it's from FreeBSD
(pre iflib)). If the problem is fixed and watchdog fired, ixgbe_init() always
calls ixgbe_jcl_reinit() and it causes panic. The reason is that
ixgbe_local_timer1(it includes watchdog function) is softint and
xgbe_jcl_reinit() calls bus_dmamem*() functions. bus_dmamem*() can't be called
from interrupt context.

  One of the way to prevent panic is use worqueue for the timer, but it's
not a small change. (I'll do it in future).

  Another way is not reallocate dmamem if it's not required. If both the MTU
(rx_mbuf_sz in reality) and the number of RX descriptors are not changed, it's
not required to call bus_dmamem_{unmap,free}(). Even if we use workque, this
change save time of ixgbe_init().

  I have a code to fix broken watchdog timer but it sometime causes watchdog
timeout, so I don't commit it yet.


To generate a diff of this commit:
cvs rdiff -u -r1.40 -r1.41 src/sys/dev/pci/ixgbe/ix_txrx.c
cvs rdiff -u -r1.45 -r1.46 src/sys/dev/pci/ixgbe/ixgbe.h
cvs rdiff -u -r1.6 -r1.7 src/sys/dev/pci/ixgbe/ixgbe_netbsd.c
cvs rdiff -u -r1.7 -r1.8 src/sys/dev/pci/ixgbe/ixgbe_netbsd.h
cvs rdiff -u -r1.21 -r1.22 src/sys/dev/pci/ixgbe/ixgbe_osdep.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.







--
---
SAITOH Masanobu (msai...@execsw.org
 msai...@netbsd.org)


Re: CVS commit: src/sys/dev/pci/ixgbe

2018-05-07 Thread Masanobu SAITOH

Hi.

On 2018/05/03 17:38, Frank Kardel wrote:

Hi,

I am now seeing that packet reception stops. tcpdump will only show outgoing 
packets and no incoming packets.

ifconfig ixgX down
ifconfig ixgX up

gets things going again.

Code from 2018-02-24 was fine and I think even code shortly before this commit 
was fine, but there was the mbuf used-after-free issue that hit me pretty hard. 
So the system wasn't running for too long anyway.

Does that ring a bell ?

Frank


 IMHO, this change doesn't cause such type of problem...

 Could you show me the following information?

 0) dmesg

This is required to know which chip is used and what type of
interrupt is used and configured (INTX, MSI or MSI-X).

 1) ifconfig -v ixg0

It's required to know offload status, the MTU size and some
error count.

 2) sysctl hw.ixg0 (or sysctl hw |grep ixg (for all ixg(4)s))

This output has some information of TX/RX ring.

 3) vmstat -e[v]

Only for ixg(4):
vmstat -e[v] |grep ixg

ixg(4) has a lot of counters. I added error counters at
all of error paths. (If you found a error path which has
no counter, it's my fault.)

Only to check checksum counters:

options INET_CSUM_COUNTERS (for ip_input())
options TCP_CSUM_COUNTERS  (for tcp_input())
options UDP_CSUM_COUNTERS  (for udp_input())
options WM_EVENT_COUNTERS  (only for wm(4))
options BGE_EVENT_COUNTERS (only for bge(4))
(No special options for ixg(4) because those counters are enabled by 
default)

vmstat -e[v] |grep csum

 4) If you use a virtual interface like vlan(4) or age(4). Please
show the configuration.


 Regards.


On 04/25/18 10:46, SAITOH Masanobu wrote:

Module Name:    src
Committed By:    msaitoh
Date:    Wed Apr 25 08:46:19 UTC 2018

Modified Files:
src/sys/dev/pci/ixgbe: ix_txrx.c ixgbe.h ixgbe_netbsd.c ixgbe_netbsd.h
    ixgbe_osdep.h

Log Message:
  Don't free and reallocate bus_dmamem when it's not required. Currently,
the watchdog timer is completely broken and never fire (it's from FreeBSD
(pre iflib)). If the problem is fixed and watchdog fired, ixgbe_init() always
calls ixgbe_jcl_reinit() and it causes panic. The reason is that
ixgbe_local_timer1(it includes watchdog function) is softint and
xgbe_jcl_reinit() calls bus_dmamem*() functions. bus_dmamem*() can't be called
from interrupt context.

  One of the way to prevent panic is use worqueue for the timer, but it's
not a small change. (I'll do it in future).

  Another way is not reallocate dmamem if it's not required. If both the MTU
(rx_mbuf_sz in reality) and the number of RX descriptors are not changed, it's
not required to call bus_dmamem_{unmap,free}(). Even if we use workque, this
change save time of ixgbe_init().

  I have a code to fix broken watchdog timer but it sometime causes watchdog
timeout, so I don't commit it yet.


To generate a diff of this commit:
cvs rdiff -u -r1.40 -r1.41 src/sys/dev/pci/ixgbe/ix_txrx.c
cvs rdiff -u -r1.45 -r1.46 src/sys/dev/pci/ixgbe/ixgbe.h
cvs rdiff -u -r1.6 -r1.7 src/sys/dev/pci/ixgbe/ixgbe_netbsd.c
cvs rdiff -u -r1.7 -r1.8 src/sys/dev/pci/ixgbe/ixgbe_netbsd.h
cvs rdiff -u -r1.21 -r1.22 src/sys/dev/pci/ixgbe/ixgbe_osdep.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.




--
---
SAITOH Masanobu (msai...@execsw.org
 msai...@netbsd.org)