shurd (Stephen Hurd)
User

Projects

User Details

User Since
Jun 19 2014, 6:57 AM (187 w, 22 h)

Recent Activity

Thu, Jan 11

shurd accepted D13837: Wider adoption of mallocarray(9)..

Looks good to me as far as bnxt and iflib are concerned.

Thu, Jan 11, 10:57 PM

Wed, Dec 27

shurd committed rS327247: Don't pass rids to taskqgroup_attach().
Don't pass rids to taskqgroup_attach()
Wed, Dec 27, 8:42 PM
shurd committed rS327244: Remove assertion that's not true for !EARLY_AP_STARTUP.
Remove assertion that's not true for !EARLY_AP_STARTUP
Wed, Dec 27, 7:14 PM
shurd committed rS327242: Fix indentation..
Fix indentation.
Wed, Dec 27, 7:12 PM

Thu, Dec 21

shurd closed D13561: Don't call tcp_lro_rx() unless hardware verified TCP/UDP csum.
Thu, Dec 21, 1:23 AM
shurd committed rS327052: Don't call tcp_lro_rx() unless hardware verified TCP/UDP csum.
Don't call tcp_lro_rx() unless hardware verified TCP/UDP csum
Thu, Dec 21, 1:22 AM

Wed, Dec 20

shurd created D13561: Don't call tcp_lro_rx() unless hardware verified TCP/UDP csum.
Wed, Dec 20, 7:56 PM

Dec 20 2017

This revision was not accepted when it landed; it landed in state Needs Review.
Dec 20 2017, 1:03 AM
shurd committed rS327013: Support attaching tx queues to cpus.
Support attaching tx queues to cpus
Dec 20 2017, 1:03 AM

Dec 19 2017

This revision was not accepted when it landed; it landed in state Needs Review.
Dec 19 2017, 10:16 PM
shurd committed rS327003: Add log messages for unknown and unhandled phy types.
Add log messages for unknown and unhandled phy types
Dec 19 2017, 10:16 PM
shurd committed rS327001: On Link up & down, update media types.
On Link up & down, update media types
Dec 19 2017, 10:06 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Dec 19 2017, 9:08 PM
shurd committed rS327000: Support short HWRM commands.
Support short HWRM commands
Dec 19 2017, 9:08 PM
shurd committed rS326999: Don't populate NVRAM sysctls for VFs.
Don't populate NVRAM sysctls for VFs
Dec 19 2017, 8:33 PM
shurd committed rS326985: Add byte swapping in bnxt_cfg_async_cr() request.
Add byte swapping in bnxt_cfg_async_cr() request
Dec 19 2017, 6:12 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Dec 19 2017, 5:59 PM
shurd committed rS326984: Update Matthew Macy contact info.
Update Matthew Macy contact info
Dec 19 2017, 5:59 PM
shurd added a comment to D12446: Support attaching tx queues to cpus.

@olivier, this should work now.

Dec 19 2017, 12:15 AM
shurd created D13537: Update Matthew Macy contact details.
Dec 19 2017, 12:11 AM

Dec 18 2017

shurd updated the diff for D12446: Support attaching tx queues to cpus.

We can't call smp_topo() more than once.

Dec 18 2017, 11:32 PM

Dec 11 2017

shurd committed rS326775: Increment encap_pad_mbuf_fail when m_dup() fails in padding.
Increment encap_pad_mbuf_fail when m_dup() fails in padding
Dec 11 2017, 8:01 PM

Dec 8 2017

shurd committed rS326706: Free mbuf chain when m_dup fails.
Free mbuf chain when m_dup fails
Dec 8 2017, 7:50 PM
shurd committed rS326702: Handle read-only mbufs in iflib ether pad function.
Handle read-only mbufs in iflib ether pad function
Dec 8 2017, 6:43 PM
shurd closed D13414: Handle read-only mbufs in ether pad function.
Dec 8 2017, 6:43 PM

Dec 7 2017

shurd created D13414: Handle read-only mbufs in ether pad function.
Dec 7 2017, 7:29 PM

Dec 6 2017

shurd abandoned D13397: Exit critical section before calling cpu_spinwait().

This is by design. We won't have two rings on the same CPU waiting for each other... we're only waiting for other CPUs to catch up.

Dec 6 2017, 5:39 PM
shurd created D13397: Exit critical section before calling cpu_spinwait().
Dec 6 2017, 5:32 PM

Dec 5 2017

shurd closed D13324: Add support to pad Ethernet frames to a minimum size.

Committed as r326578 (which had the wrong Differential Revision of D13269 specified).

Dec 5 2017, 9:03 PM
shurd reopened D13269: bnxt: Add SR-IOV support for Stratus 100G NIC.

Wrong URL pasted in D13324 commit. :-(

Dec 5 2017, 9:02 PM
shurd closed D13269: bnxt: Add SR-IOV support for Stratus 100G NIC.
Dec 5 2017, 9:00 PM
shurd committed rS326578: iflib: Support to padding Ethernet frames to a min size.
iflib: Support to padding Ethernet frames to a min size
Dec 5 2017, 9:00 PM
shurd committed rS326577: Avoid calling CURVNET_[SET|RESTORE] for each packet.
Avoid calling CURVNET_[SET|RESTORE] for each packet
Dec 5 2017, 8:43 PM
shurd closed D13368: Avoid calling CURVNET_[SET|RESTORE] up to twice per packet.
Dec 5 2017, 8:43 PM
shurd added a comment to D13368: Avoid calling CURVNET_[SET|RESTORE] up to twice per packet.

Updated, thanks.

Dec 5 2017, 6:48 PM
shurd updated the diff for D13368: Avoid calling CURVNET_[SET|RESTORE] up to twice per packet.

Integrate feedback from ae and cem

Dec 5 2017, 6:48 PM
shurd added inline comments to D13358: bnxt: Correct the logic to report supported speeds.
Dec 5 2017, 6:36 PM

Dec 4 2017

shurd created D13368: Avoid calling CURVNET_[SET|RESTORE] up to twice per packet.
Dec 4 2017, 8:37 PM
shurd updated the diff for D13324: Add support to pad Ethernet frames to a minimum size.

Mark iflib_ether_pad() as noinline and move device_printf() into it

Dec 4 2017, 7:51 PM
shurd updated the diff for D13324: Add support to pad Ethernet frames to a minimum size.

Move ether padding into spearate function, predict_false() the length test

Dec 4 2017, 7:43 PM
shurd added inline comments to D13358: bnxt: Correct the logic to report supported speeds.
Dec 4 2017, 7:10 PM
shurd added a comment to D13269: bnxt: Add SR-IOV support for Stratus 100G NIC.

Looks good, but I don't really like the title. I'm thinking of a commit message more like the following:

Dec 4 2017, 6:58 PM
shurd updated the diff for D13324: Add support to pad Ethernet frames to a minimum size.

Return an error of m_append() fails, not success.

Dec 4 2017, 6:38 PM

Dec 1 2017

shurd added a comment to D12446: Support attaching tx queues to cpus.

Without witness it panics too:

#11 linux_queue_work_on (cpu=256, wq=0x202000000000002, work=0xfffff80200000058) at /usr/src/sys/compat/linuxkpi/common/src/linux_work.c:139
#12 0xffffffff80cdf95d in inetaddr_event (this=<optimized out>, event=<optimized out>, ptr=0xfffff801c7ff6800) at /usr/src/sys/compat/linuxkpi/common/include/asm/atomic.h:75
Dec 1 2017, 7:49 PM
shurd updated the diff for D13324: Add support to pad Ethernet frames to a minimum size.

Move BNXT_MIN_FRAME_SIZE into bnxt.h where D13269 defined BNXT_MIN_PKT_SIZE

Dec 1 2017, 7:44 PM
shurd added a comment to D13269: bnxt: Add SR-IOV support for Stratus 100G NIC.

I just thought of another possibility... the hardware may only be able to pad frames that are 52 bytes or larger, but it still pads them out to 64 bytes... so basically the "padding offload" feature is broken on these chips. This would mean that legal minimum sized frames can be transmitted etc, just that they need special preparation. This still means an iflib quirk thing, and additional processing when transmitting small packets (where it hurts the worst), but at least all legal frames can still be sent. This seems to match up with your patch too.

Dec 1 2017, 6:58 PM
shurd created D13324: Add support to pad Ethernet frames to a minimum size.
Dec 1 2017, 6:55 PM
shurd added a comment to D12897: bnxt: Workaround for ifconfig showing media as 'Other'.

So with D13312 committed, is there still a need for a change? If not, please abandon this review.

Dec 1 2017, 5:59 PM
shurd closed D13312: Add support for SIOCGIFXMEDIA to iflib.
Dec 1 2017, 5:58 PM
shurd committed rS326432: Add support for SIOCGIFXMEDIA to iflib.
Add support for SIOCGIFXMEDIA to iflib
Dec 1 2017, 5:58 PM
shurd added a comment to D13269: bnxt: Add SR-IOV support for Stratus 100G NIC.

How are you testing this padding?

<Chenna> ARP response pkt is less than 60 bytes, right? I'm able to repro using ARP response pkt only.

The ARP packet itself is 28 bytes, yes... but the Ethernet packet which contains it will pad the payload out to 46 bytes (42 if there's a single VLAN tag), making the Ethernet frame 64 bytes and the Ethernet packet will be 72 bytes. Are you saying the Ethernet *payload* has a minimum limit of 52 bytes on some controllers? Or is it that there's a minimum frame size of 70 bytes?

Dec 1 2017, 5:57 AM
shurd added a comment to D13269: bnxt: Add SR-IOV support for Stratus 100G NIC.

How are you testing this padding?

<Chenna> ARP response pkt is less than 60 bytes, right? I'm able to repro using ARP response pkt only.

Dec 1 2017, 5:41 AM

Nov 30 2017

shurd added inline comments to D13269: bnxt: Add SR-IOV support for Stratus 100G NIC.
Nov 30 2017, 9:56 PM
shurd added a comment to D12897: bnxt: Workaround for ifconfig showing media as 'Other'.

In brief: I still see the problem as ifconfig still displaying "Other"

ifmr->ifm_current: 0x3f

Nov 30 2017, 7:57 PM
shurd created D13312: Add support for SIOCGIFXMEDIA to iflib.
Nov 30 2017, 7:51 PM

Nov 29 2017

shurd updated the diff for D12446: Support attaching tx queues to cpus.

Fix non-SMP build

Nov 29 2017, 7:32 PM
shurd updated the diff for D12446: Support attaching tx queues to cpus.

Add missing #ifdef

Nov 29 2017, 6:23 PM
shurd committed rS326370: Fix comment introduced in r326369.
Fix comment introduced in r326369
Nov 29 2017, 6:21 PM
shurd updated the diff for D12446: Support attaching tx queues to cpus.

The ctx->ifc_cpus initialization change has been committed as
r326369, update patch to remove that change.

Nov 29 2017, 6:17 PM
shurd committed rS326369: Ensure that ctx->ifc_cpus is always initialized.
Ensure that ctx->ifc_cpus is always initialized
Nov 29 2017, 6:15 PM
shurd added a reviewer for D12446: Support attaching tx queues to cpus: mjg.

Adding mjg, since

Nov 29 2017, 6:04 PM

Nov 28 2017

shurd added a comment to D12897: bnxt: Workaround for ifconfig showing media as 'Other'.

Hi Stephen,

Ah, you likely need to clear IFM_ETH_FMASK as well.

<Chenna>
Can you please confirm if this is what you are suggesting?

if (softc->link_info.autoneg & BNXT_AUTONEG_SPEED) {
         ifmedia_set(softc->media, IFM_ETHER | IFM_AUTO);
} else {
         ifmedia_set(softc->media, ifmr->ifm_active & ~(IFM_GMASK|IFM_ETH_FMASK));
}

But, I still see Crash with that code, Please find attached screenshot.

Nov 28 2017, 7:46 PM
shurd added a comment to D12446: Support attaching tx queues to cpus.

The rw lock stuff looks stable in HEAD now, so this should be testable against it.

Nov 28 2017, 7:43 PM

Nov 27 2017

shurd added a comment to D13269: bnxt: Add SR-IOV support for Stratus 100G NIC.

It's not clear how this change "Add[s] SR-IOV support for Stratus 100G NIC". A better description should be added.

Nov 27 2017, 9:17 PM

Nov 23 2017

shurd added a comment to D12446: Support attaching tx queues to cpus.

Do you known working revision I can use ?

Nov 23 2017, 6:37 PM

Nov 22 2017

shurd added a comment to D12446: Support attaching tx queues to cpus.

I've got a new panic (head with WITNESS and INVARIANTS enabled):

panic: Lock (rw) ifnet_rw not locked @ /usr/local/BSDRP/TESTING/FreeBSD/src/sys/net/if.c:262.

It's this code (prefixed with line number):

260     IFNET_RLOCK_NOSLEEP();
261     ifp = ifnet_byindex_locked(idx);
262     IFNET_RUNLOCK_NOSLEEP();
Nov 22 2017, 9:32 PM

Nov 21 2017

shurd updated the diff for D12446: Support attaching tx queues to cpus.

Ensure ifc_cpus is always initialized, even when msix is not used.

Nov 21 2017, 9:27 PM

Nov 20 2017

shurd added a comment to D12446: Support attaching tx queues to cpus.

And you try this after r326033? These issues could have been caused by the memory corruption fixed in that commit.

Nov 20 2017, 9:59 PM
shurd committed rS326033: Fix off-by-one error in bit_nclear() usage.
Fix off-by-one error in bit_nclear() usage
Nov 20 2017, 9:57 PM

Nov 17 2017

shurd added a comment to D12446: Support attaching tx queues to cpus.

How to dump a panic when the kernel crash during boot before loading disk controller drivers ?
Can I compile a kernel with .debug embedded into the kernel ?

Nov 17 2017, 6:04 PM
shurd added a comment to D12446: Support attaching tx queues to cpus.

Once I've applied this patch on my system (that is already patched with D11727 and D13096) it panic.

spin lock 0xffffffff81d916b0 ((null)) held by 0xffffffff81d91960 (tid 0) too long
panic: spin lock held too long
cpuid = 22
time = 1
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xffffffff821cff80
vpanic() at vpanic+0x19c/frame 0xffffffff821d0000
panic() at panic+0x43/frame 0xffffffff821d0060
_mtx_lock_spin_cookie() at _mtx_lock_spin_cookie+0x339/frame 0xffffffff821d00d0
turnstile_trywait() at turnstile_trywait+0xd3/frame 0xffffffff821d0100
__mtx_lock_sleep() at __mtx_lock_sleep+0xd9/frame 0xffffffff821d0190
bpfattach2() at bpfattach2+0x146/frame 0xffffffff821d01d0
ether_ifattach() at ether_ifattach+0xe4/frame 0xffffffff821d0210
iflib_device_register() at iflib_device_register+0x2706/frame 0xffffffff821d0550
iflib_device_attach() at iflib_device_attach+0xb7/frame 0xffffffff821d0580
device_attach() at device_attach+0x3f5/frame 0xffffffff821d05d0
bus_generic_attach() at bus_generic_attach+0x5c/frame 0xffffffff821d0600
pci_attach() at pci_attach+0xd5/frame 0xffffffff821d0640
device_attach() at device_attach+0x3f5/frame 0xffffffff821d0690
bus_generic_attach() at bus_generic_attach+0x5c/frame 0xffffffff821d06c0
acpi_pcib_pci_attach() at acpi_pcib_pci_attach+0xa1/frame 0xffffffff821d0700
device_attach() at device_attach+0x3f5/frame 0xffffffff821d0750
bus_generic_attach() at bus_generic_attach+0x5c/frame 0xffffffff821d0780
pci_attach() at pci_attach+0xd5/frame 0xffffffff821d07c0
device_attach() at device_attach+0x3f5/frame 0xffffffff821d0810
bus_generic_attach() at bus_generic_attach+0x5c/frame 0xffffffff821d0840
acpi_pcib_acpi_attach() at acpi_pcib_acpi_attach+0x3bc/frame 0xffffffff821d08b0
device_attach() at device_attach+0x3f5/frame 0xffffffff821d0900
bus_generic_attach() at bus_generic_attach+0x5c/frame 0xffffffff821d0930
acpi_attach() at acpi_attach+0xe85/frame 0xffffffff821d09e0
device_attach() at device_attach+0x3f5/frame 0xffffffff821d0a30
bus_generic_attach() at bus_generic_attach+0x5c/frame 0xffffffff821d0a60
nexus_acpi_attach() at nexus_acpi_attach+0x73/frame 0xffffffff821d0a90
device_attach() at device_attach+0x3f5/frame 0xffffffff821d0ae0
bus_generic_new_pass() at bus_generic_new_pass+0x118/frame 0xffffffff821d0b10
root_bus_configure() at root_bus_configure+0x77/frame 0xffffffff821d0b40
configure() at configure+0x9/frame 0xffffffff821d0b50
mi_startup() at mi_startup+0x9c/frame 0xffffffff821d0b70
btext() at btext+0x2c
KDB: enter: panic
[ thread pid 0 tid 100000 ]
Stopped at      kdb_enter+0x3b: movq    $0,kdb_why

Do you want a panic dump with debug symbol ?

Nov 17 2017, 5:21 PM

Nov 16 2017

shurd committed rS325901: Fix default numbers of iflib queue sets.
Fix default numbers of iflib queue sets
Nov 16 2017, 6:53 PM
shurd closed D13096: Fix default numbers of queue sets.
Nov 16 2017, 6:53 PM

Nov 15 2017

shurd added a comment to D12446: Support attaching tx queues to cpus.

Once I've applied this patch on my system (that is already patched with D11727 and D13096) it panic.

Do you want a panic dump with debug symbol ?

Nov 15 2017, 9:21 PM
shurd added a comment to D11727: ixgbe(4): Update HEAD to use iflib.

with D13096 I've got only 12 queues assigned, no more NUMA mess :-)

ix0: <Intel(R) PRO/10GbE PCI-Express Network Driver> port 0x2020-0x203f mem 0x91d00000-0x91dfffff,0x91e04000-0x91e07fff irq 44 at device 0.0 numa-domain 0 on pci5
ix0: using 2048 tx descriptors and 2048 rx descriptors
ix0: msix_init qsets capped at 32
ix0: pxm cpus: 12 queue msgs: 63 admincnt: 1
ix0: using 12 rx queues 12 tx queues
ix0: Using MSIX interrupts with 13 vectors
ix0: allocated for 12 queues
ix0: allocated for 12 rx queues
ix0: Ethernet address: 24:6e:96:5b:92:80
ix0: PCI Express Bus: Speed 5.0GT/s Width x8
ix0: netmap queues/slots: TX 12/2048, RX 12/2048

But queue seems not bound to core (they jump to other cores, creating large standard derivation on the bench result), then I had to use an affinity script for best benefit:

Nov 15 2017, 6:53 PM

Nov 14 2017

shurd updated the diff for D12446: Support attaching tx queues to cpus.

Update patch to current HEAD

Nov 14 2017, 10:09 PM
shurd added a comment to D11727: ixgbe(4): Update HEAD to use iflib.

Are you sure about the number of queues? For iflib version of driver the default number is based on number of cores. Are you setting dev.ix.X.override_n(r|t)xqs sysctls?

Let me check on my /var/run/dmesg:

ix0: msix_init qsets capped at 32
ix0: pxm cpus: 12 queue msgs: 63 admincnt: 1
ix0: using 24 rx queues 24 tx queues

oops, you've right… it uses all the 24 cores :-(
Notice that I'm using 2000 UDP flows (20 sources IP and 100 destinations IP), and queues 16 to 23 seems not used.

Then should I force it to use only 12 queues for my use case for avoiding NUMA mess ?

Nov 14 2017, 9:54 PM
shurd created D13096: Fix default numbers of queue sets.
Nov 14 2017, 9:52 PM
shurd added a comment to D11727: ixgbe(4): Update HEAD to use iflib.

Are you sure about the number of queues? For iflib version of driver the default number is based on number of cores. Are you setting dev.ix.X.override_n(r|t)xqs sysctls?

oops, you've right… it uses all the 24 cores :-(
Notice that I'm using 2000 UDP flows (20 sources IP and 100 destinations IP), and queues 16 to 23 seems not used.

Then should I force it to use only 12 queues for my use case for avoiding NUMA mess ?

Nov 14 2017, 5:52 PM

Nov 13 2017

shurd added a comment to D11727: ixgbe(4): Update HEAD to use iflib.

Second, on a Dual CPU, Xeon_E5-2650 (12Cores), with Intel 82599ES 10Gigabit (using default 8 queues):

Nov 13 2017, 7:50 PM

Nov 7 2017

shurd committed rS325487: Only chain non-LRO mbufs when LRO is not possible.
Only chain non-LRO mbufs when LRO is not possible
Nov 7 2017, 3:51 PM
shurd committed rS325488: bnxt: Add support for new phy_types and speeds - Part #2.
bnxt: Add support for new phy_types and speeds - Part #2
Nov 7 2017, 3:51 PM
shurd closed D12876: Only chain non-LRO mbufs when LRO is not possible.

committed as r325487

Nov 7 2017, 3:51 PM
shurd closed D12896: bnxt: Add support for new phy_types and speeds - Part #2.

committed as r325488

Nov 7 2017, 3:51 PM
shurd added a reviewer for D12976: Fix issue with VXLAN-encapsulated rx h/w checksum: bhargava.marreddy_broadcom.com.

Adding Chenna.

Nov 7 2017, 3:51 PM
shurd added a comment to D12897: bnxt: Workaround for ifconfig showing media as 'Other'.

Ah, you likely need to clear IFM_ETH_FMASK as well. I think that should be part of the default ifm_mask, but masking that off here should be good for now.

Nov 7 2017, 3:51 PM

Nov 3 2017

shurd added a comment to D12897: bnxt: Workaround for ifconfig showing media as 'Other'.

Ah right, you likely need to mask off IFM_GMASK to remove the duplex/flow control bits as well... ifmedia_set(softc->media, ifmr->ifm_active & ~IFM_GMASK);

Nov 3 2017, 7:22 PM
shurd accepted D12896: bnxt: Add support for new phy_types and speeds - Part #2.

lgtm

Nov 3 2017, 7:11 PM

Nov 2 2017

shurd updated the diff for D12876: Only chain non-LRO mbufs when LRO is not possible.

Fix typo

Nov 2 2017, 6:33 PM
shurd requested changes to D12896: bnxt: Add support for new phy_types and speeds - Part #2.

This seems to be a very complex and fragile way of looking up the supported media type by speed. Why not just loop through ifm_list looking for a matching speed? Something like this:

Nov 2 2017, 5:21 PM

Nov 1 2017

shurd requested changes to D12897: bnxt: Workaround for ifconfig showing media as 'Other'.

This looks like it would break the autoselect indication.

Nov 1 2017, 4:54 PM

Oct 31 2017

shurd closed D12880: Preserve TSO checksum flags.
Oct 31 2017, 7:03 PM
shurd committed rS325245: Preserve TSO checksum flags.
Preserve TSO checksum flags
Oct 31 2017, 7:03 PM
shurd updated the diff for D12876: Only chain non-LRO mbufs when LRO is not possible.

Much better way of dropping a level of indentation. :-)

Oct 31 2017, 6:57 PM
shurd created D12880: Preserve TSO checksum flags.
Oct 31 2017, 6:46 PM
shurd added inline comments to D12876: Only chain non-LRO mbufs when LRO is not possible.
Oct 31 2017, 6:08 PM
shurd updated the diff for D12876: Only chain non-LRO mbufs when LRO is not possible.

Incorporate feedback.

Oct 31 2017, 6:07 PM
shurd committed rS325241: Fix PR221990 - Assertion at iflib.c:1947.
Fix PR221990 - Assertion at iflib.c:1947
Oct 31 2017, 5:50 PM
shurd closed D12798: Possible fix for PR221990.
Oct 31 2017, 5:50 PM
shurd updated the summary of D12876: Only chain non-LRO mbufs when LRO is not possible.
Oct 31 2017, 5:38 PM