Page MenuHomeFreeBSD

jacob.e.keller_intel.com (Jacob Keller)
User

Projects

User Details

User Since
Aug 24 2017, 3:21 PM (107 w, 3 d)

Recent Activity

Mon, Sep 9

jacob.e.keller_intel.com added a comment to D21547: ix, ixv: Read msix_bar from device configuration.

I agree with Eric, we should depend on the device to be configured properly, and push to get the images for the device updated if they're wrong.

Mon, Sep 9, 6:01 PM

Thu, Sep 5

jacob.e.keller_intel.com added a comment to D21540: initialize the STATE_LOCK in iflib_register.

This is specifically a fix for 11-STABLE. It's already fixed in CURRENT and STABLE-12. It looks like it was an accidental miss in MFC for STABLE-11 a few months ago.

Thu, Sep 5, 10:33 PM
jacob.e.keller_intel.com added a reviewer for D21540: initialize the STATE_LOCK in iflib_register: erj.
Thu, Sep 5, 10:33 PM
jacob.e.keller_intel.com created D21540: initialize the STATE_LOCK in iflib_register.
Thu, Sep 5, 10:32 PM

Aug 13 2019

jacob.e.keller_intel.com added a comment to D21240: net: add ETHER_IS_ZERO macro similar to ETHER_IS_BROADCAST.

It’s seems I found another incorrect solution:
#include <stdio.h>
int main()
{

unsigned char a = (1 << 0) ||(1<<1)||(1 <<2)||(1<<3)||(1<<4)||(1<<5);
printf("momentum more : %d\n", a);
return 0;

}

Aug 13 2019, 11:15 PM
jacob.e.keller_intel.com added a comment to D21240: net: add ETHER_IS_ZERO macro similar to ETHER_IS_BROADCAST.

Use bitwise OR instead of bitwise AND to correct implementation

Aug 13 2019, 9:21 PM
jacob.e.keller_intel.com added inline comments to D21240: net: add ETHER_IS_ZERO macro similar to ETHER_IS_BROADCAST.
Aug 13 2019, 9:19 PM

Aug 12 2019

jacob.e.keller_intel.com created D21240: net: add ETHER_IS_ZERO macro similar to ETHER_IS_BROADCAST.
Aug 12 2019, 10:53 PM
jacob.e.keller_intel.com created D21239: net: prefer ETHER_ADDR_LEN over ETH_ADDR_LEN.
Aug 12 2019, 10:53 PM

Jul 31 2019

jacob.e.keller_intel.com added a comment to D21005: iflib: add iflib_deregister to help cleanup on exit.

Move the kobject refcount removal to a separate patch

Jul 31 2019, 7:49 PM
jacob.e.keller_intel.com created D21125: iflib: remove kobject class reference increment.
Jul 31 2019, 7:49 PM

Jul 23 2019

jacob.e.keller_intel.com removed 1 blocking reviewer(s) for D21003: iflib: fix dangling device softc pointer: Intel Networking.
Jul 23 2019, 10:06 PM
jacob.e.keller_intel.com added a comment to D21005: iflib: add iflib_deregister to help cleanup on exit.
In D21005#456595, @erj wrote:

This is built on top of https://reviews.freebsd.org/D21004
It could probably be backported, but isn't as necessary as the previous patch which is why I kept them separate.

Do you mean https://reviews.freebsd.org/D21003?

Jul 23 2019, 8:18 PM

Jul 19 2019

jacob.e.keller_intel.com added a reviewer for D21005: iflib: add iflib_deregister to help cleanup on exit: erj.
Jul 19 2019, 11:53 PM
jacob.e.keller_intel.com added a comment to D21005: iflib: add iflib_deregister to help cleanup on exit.

This is built on top of https://reviews.freebsd.org/D21003

Jul 19 2019, 11:53 PM
jacob.e.keller_intel.com created D21005: iflib: add iflib_deregister to help cleanup on exit.
Jul 19 2019, 11:52 PM
jacob.e.keller_intel.com added a reviewer for D21003: iflib: fix dangling device softc pointer: erj.
Jul 19 2019, 10:46 PM
jacob.e.keller_intel.com updated the test plan for D21003: iflib: fix dangling device softc pointer.
Jul 19 2019, 10:45 PM
jacob.e.keller_intel.com created D21003: iflib: fix dangling device softc pointer.
Jul 19 2019, 10:44 PM

May 10 2019

jacob.e.keller_intel.com added a comment to D20221: iflib: provide probe wrapper for vendor drivers.

We discovered this as part of our rename from ixlv to iavf.

May 10 2019, 12:17 AM
jacob.e.keller_intel.com created D20221: iflib: provide probe wrapper for vendor drivers.
May 10 2019, 12:15 AM

Apr 25 2019

jacob.e.keller_intel.com added a comment to D19880: iflib: use default ntxd and nrxd when user value is not power of 2.

Ok, I updated this revision to just use the default value now instead of rounding down.

Apr 25 2019, 8:52 PM
jacob.e.keller_intel.com retitled D19880: iflib: use default ntxd and nrxd when user value is not power of 2 from iflib: round down ntxd and nrxd when not power of 2 to iflib: use default ntxd and nrxd when user value is not power of 2.
Apr 25 2019, 8:50 PM
jacob.e.keller_intel.com updated the diff for D19880: iflib: use default ntxd and nrxd when user value is not power of 2.

use the default value instead of rounding down

Apr 25 2019, 8:41 PM

Apr 22 2019

jacob.e.keller_intel.com updated the diff for D19880: iflib: use default ntxd and nrxd when user value is not power of 2.

Modify _iflib_assert to verify driver parameters are power of 2

Apr 22 2019, 4:58 PM
jacob.e.keller_intel.com added a comment to D19880: iflib: use default ntxd and nrxd when user value is not power of 2.

I would be fine modifying the iflib code to avoid the power of 2 limitation entirely, but I don't think there is significant gain for that, because we would have to add a configuration knob indicating the increment. Additionally, since some hardware likely does require power-of-2 values, we'd want to have a way for those drivers to express that restriction as well. power-of-2 restriction can't be expressed with just an increment value, so that becomes a lot trickier.

Apr 22 2019, 4:39 PM
jacob.e.keller_intel.com added a comment to D19880: iflib: use default ntxd and nrxd when user value is not power of 2.

Yea, I'll add MPASS to check that the values are power of 2.

Apr 22 2019, 4:37 PM

Apr 11 2019

jacob.e.keller_intel.com added a reviewer for D19880: iflib: use default ntxd and nrxd when user value is not power of 2: erj.
Apr 11 2019, 5:50 PM
jacob.e.keller_intel.com added a comment to D19880: iflib: use default ntxd and nrxd when user value is not power of 2.

I have a couple of thoughts I had while writing this.

Apr 11 2019, 5:50 PM

Apr 10 2019

jacob.e.keller_intel.com created D19880: iflib: use default ntxd and nrxd when user value is not power of 2.
Apr 10 2019, 10:44 PM

Mar 19 2019

jacob.e.keller_intel.com added a reviewer for D19652: iflib: return ENETDOWN when the network device is down: sbruno.
Mar 19 2019, 11:38 PM
jacob.e.keller_intel.com created D19652: iflib: return ENETDOWN when the network device is down.
Mar 19 2019, 11:38 PM

Mar 15 2019

jacob.e.keller_intel.com created D19604: iflib: hold the CTX lock in iflib_pseudo_register.
Mar 15 2019, 11:53 PM
jacob.e.keller_intel.com updated the diff for D19577: iflib: mark isc_driver_version as constant.

Document the new CONST_STRING sysctl macros

Mar 15 2019, 12:38 AM

Mar 14 2019

jacob.e.keller_intel.com added inline comments to D19577: iflib: mark isc_driver_version as constant.
Mar 14 2019, 11:42 PM
jacob.e.keller_intel.com added a comment to D19577: iflib: mark isc_driver_version as constant.

I like the addition of SYSCTL_ADD_CONST_STRING since it also ensures that we enforce that we don't have CTLFLAG_WR.

Mar 14 2019, 11:42 PM
jacob.e.keller_intel.com updated the diff for D19577: iflib: mark isc_driver_version as constant.

Use __DECONST instead of a poor reimplementation

Mar 14 2019, 11:40 PM
jacob.e.keller_intel.com added a comment to D19577: iflib: mark isc_driver_version as constant.
In D19577#419330, @jhb wrote:

I think I'd rather have a SYSCTL_ADD_CONST_STRING or the like, but presumably you'd just get the warning again. The other hack is to cast through uintptr_t and it's what we normally do via the __DECONST() macro. I would just do that unconditionally perhaps:

char *__arg = __DECONST(arg, char *);
Mar 14 2019, 11:19 PM
jacob.e.keller_intel.com added inline comments to D19577: iflib: mark isc_driver_version as constant.
Mar 14 2019, 10:42 PM
jacob.e.keller_intel.com added a comment to D19577: iflib: mark isc_driver_version as constant.

This version actually compilers without the warnings, but it *is* rather ugly to have to do it this way

Mar 14 2019, 10:41 PM
jacob.e.keller_intel.com updated the diff for D19577: iflib: mark isc_driver_version as constant.

Introduce a __drop_const macro to handle the warning

Mar 14 2019, 10:40 PM
Herald added a reviewer for D19577: iflib: mark isc_driver_version as constant: iflib.

Looks like the best 'fix' would be to use _Pragma to drop the warnings in SYSCTL_ADD_STRING. That will take a bit more work to get correct.

Mar 14 2019, 9:02 PM
jacob.e.keller_intel.com added inline comments to D19468: iflib: prevent possible infinite loop in iflib_encap.
Mar 14 2019, 8:59 PM
jacob.e.keller_intel.com added a comment to D19577: iflib: mark isc_driver_version as constant.

Looks like the best 'fix' would be to use _Pragma to drop the warnings in SYSCTL_ADD_STRING. That will take a bit more work to get correct.

Mar 14 2019, 12:14 AM
jacob.e.keller_intel.com planned changes to D19577: iflib: mark isc_driver_version as constant.

This still doesn't quite work 100%, because it causes a -Wcast-qual warning....

Mar 14 2019, 12:03 AM
jacob.e.keller_intel.com updated the diff for D19577: iflib: mark isc_driver_version as constant.

Also cast isc_driver_version in SYSCTL_ADD_STRING

Mar 14 2019, 12:03 AM

Mar 13 2019

jacob.e.keller_intel.com planned changes to D19577: iflib: mark isc_driver_version as constant.

This needs some re-work before it's acceptable.

Mar 13 2019, 11:44 PM
jacob.e.keller_intel.com added a comment to D19577: iflib: mark isc_driver_version as constant.

Uhh, oh I see why it's marked as non-const.... SYSCTL_ADD_STRING with a read-only flag doesn't know to take a const value....

Mar 13 2019, 11:42 PM
jacob.e.keller_intel.com added a comment to D19577: iflib: mark isc_driver_version as constant.

Drivers can sort of get around this by using an explicit cast, but that triggers a -Wcast-qual warning.

Mar 13 2019, 10:43 PM
jacob.e.keller_intel.com added reviewers for D19577: iflib: mark isc_driver_version as constant: erj, gallatin.
Mar 13 2019, 10:42 PM
jacob.e.keller_intel.com created D19577: iflib: mark isc_driver_version as constant.
Mar 13 2019, 10:41 PM
jacob.e.keller_intel.com added inline comments to D19489: iflib: expose the Rx mbuf buffer size to drivers.
Mar 13 2019, 7:12 PM
jacob.e.keller_intel.com updated the diff for D19489: iflib: expose the Rx mbuf buffer size to drivers.

Replace the 2048 magic number with MCLBYTES

Mar 13 2019, 7:12 PM
jacob.e.keller_intel.com added a comment to D19468: iflib: prevent possible infinite loop in iflib_encap.

I think this is a mostly theoretical issue, as you'll have had a chain which was impossible to send (eg, far too large, etc) even after defrag.

Mar 13 2019, 7:10 PM
jacob.e.keller_intel.com updated the diff for D19468: iflib: prevent possible infinite loop in iflib_encap.

add an MPASS so we trigger a panic when in debug

Mar 13 2019, 7:05 PM

Mar 8 2019

jacob.e.keller_intel.com updated the diff for D19489: iflib: expose the Rx mbuf buffer size to drivers.

Fix minor style nits

Mar 8 2019, 5:55 PM

Mar 7 2019

jacob.e.keller_intel.com added reviewers for D19468: iflib: prevent possible infinite loop in iflib_encap: Intel Networking, erj.
Mar 7 2019, 12:24 AM
jacob.e.keller_intel.com added a comment to D19489: iflib: expose the Rx mbuf buffer size to drivers.

The intention here is to ensure that device drivers which need the size of the rx buffers can ensure they use the same size as iflib, (possibly incase we want to re-enable something like the old CONTIG_MALLOCWORKS again)

Mar 7 2019, 12:11 AM
jacob.e.keller_intel.com added reviewers for D19489: iflib: expose the Rx mbuf buffer size to drivers: Intel Networking, erj.
Mar 7 2019, 12:10 AM
jacob.e.keller_intel.com created D19489: iflib: expose the Rx mbuf buffer size to drivers.
Mar 7 2019, 12:09 AM

Mar 5 2019

jacob.e.keller_intel.com added a comment to D19468: iflib: prevent possible infinite loop in iflib_encap.

I'm definitely open to better/alternative solutions, and no, I don't actually have a live replication of the issue.

Mar 5 2019, 7:20 PM
jacob.e.keller_intel.com added inline comments to D19468: iflib: prevent possible infinite loop in iflib_encap.
Mar 5 2019, 7:18 PM
jacob.e.keller_intel.com updated the diff for D19468: iflib: prevent possible infinite loop in iflib_encap.

Move the remap++ to after the error check

Mar 5 2019, 7:18 PM
jacob.e.keller_intel.com added a comment to D19468: iflib: prevent possible infinite loop in iflib_encap.

I found this during code inspection while looking at a similar construction for a legacy device driver. It's possible this can never trigger, depending on how bus_dmamap_load_mbuf_sg returns EFBIG, but it definitely seemed plausible that it might happen, and potentially locking up a CPU is not a happy thing to do.

Mar 5 2019, 7:14 PM
jacob.e.keller_intel.com created D19468: iflib: prevent possible infinite loop in iflib_encap.
Mar 5 2019, 7:12 PM

Feb 14 2019

jacob.e.keller_intel.com added a comment to D19199: remove references to CONTIGMALLOC_WORKS in iflib.

I think it would be useful to actually expose the ifl_buf_size calculation somehow, since hardware needs this information. Currently, most drivers just calculate the value in the same way that iflib does, but that seems error prone if we ever do change this in the future.

Feb 14 2019, 6:26 PM
jacob.e.keller_intel.com created D19199: remove references to CONTIGMALLOC_WORKS in iflib.
Feb 14 2019, 6:20 PM

Jan 28 2019

jacob.e.keller_intel.com added inline comments to D18980: Stop iflib(4) from leaking MSI messages and along with drivers let it use the correct RIDs when releasing resources.
Jan 28 2019, 7:09 PM

Dec 19 2018

jacob.e.keller_intel.com added a comment to D18470: ix(4),ixv(4): Fix TSO offloads when TXCSUM is disabled.

We probably should only check CSUM_TSO for TCP packets..

Dec 19 2018, 8:33 PM
jacob.e.keller_intel.com added a comment to D18470: ix(4),ixv(4): Fix TSO offloads when TXCSUM is disabled.

I think I see what's wrong... we check CSUM_TSO for IPPROTO_UDP and IPPROTO_SCTP. That doesn't make sense, since TSO is only for TCP

Dec 19 2018, 8:30 PM
jacob.e.keller_intel.com added a comment to D18470: ix(4),ixv(4): Fix TSO offloads when TXCSUM is disabled.

We are seeing tx hangs with UDP using multiple clients when offloads are disabled. Cannot reproduce on 12.0-RELEASE. Reproducible on X520 and X550.

Dec 19 2018, 7:40 PM

Dec 17 2018

jacob.e.keller_intel.com added a comment to D18545: intel: correct initialization of tx_cidx_processed.

The following pkt-gen command:

Dec 17 2018, 9:57 PM
jacob.e.keller_intel.com added a comment to D18545: intel: correct initialization of tx_cidx_processed.

I'm not entirely certain how to reproduce the original setup that triggered the bug, so some help with that would be appreciated.

The easiest way to trigger the issue is to use net/pkt-gen to send a fixed number of packets. With the bug, pkt-gen would never complete because it wasn't notified that all packets were sent.

Dec 17 2018, 9:39 PM

Dec 14 2018

jacob.e.keller_intel.com added a comment to D18545: intel: correct initialization of tx_cidx_processed.

Stephen Hurd, this is what I believe to be the correct fix for this issue. I'm not entirely certain how to reproduce the original setup that triggered the bug, so some help with that would be appreciated.

Dec 14 2018, 12:04 AM
jacob.e.keller_intel.com created D18545: intel: correct initialization of tx_cidx_processed.
Dec 14 2018, 12:00 AM

Dec 6 2018

jacob.e.keller_intel.com added inline comments to D18368: Fix first-packet completion.
Dec 6 2018, 11:10 PM
jacob.e.keller_intel.com added a comment to D18368: Fix first-packet completion.

I know I'm late commenting on this, but I think the correct fix is to initialize tx_cidx_processed to txr->desc_count -1 when we initialize.

Dec 6 2018, 11:10 PM

Nov 7 2018

jacob.e.keller_intel.com added reviewers for D17900: ixl/iavf: fix TSO offloads when TXCSUM is disabled: Intel Networking, shurd, erj.
Nov 7 2018, 11:56 PM
jacob.e.keller_intel.com created D17900: ixl/iavf: fix TSO offloads when TXCSUM is disabled.
Nov 7 2018, 11:54 PM
jacob.e.keller_intel.com added inline comments to D17881: Fix rxcsum issue introduced in r338838.
Nov 7 2018, 9:31 PM
jacob.e.keller_intel.com added a comment to D17881: Fix rxcsum issue introduced in r338838.

@shurd - Turns out you've been faster. We've been working one this bug too. Our plan was to do it this way:

			if ((setmask & (IFCAP_RXCSUM | IFCAP_RXCSUM_IPV6)) !=
			    (oldmask & (IFCAP_RXCSUM | IFCAP_RXCSUM_IPV6)))
				setmask |= IFCAP_RXCSUM | IFCAP_RXCSUM_IPV6;

But it's too late for me today to figure out which has more sense ;)

Nov 7 2018, 9:29 PM

Nov 5 2018

jacob.e.keller_intel.com added a comment to D17801: TSO inherently requires checksum offload. In the case where TSO is in use, force the checksum offload on as well for that packet..

If I fix the driver to enable IP checksums on TSOs even when CSUM_IP isn't set, *and* I enable IFLIB_TSO_INIT_IP, then this patch isn't necessary.

Nov 5 2018, 11:12 PM
jacob.e.keller_intel.com added a comment to D17801: TSO inherently requires checksum offload. In the case where TSO is in use, force the checksum offload on as well for that packet..

Actually, I think the reason we set NEED_CSUM_ZERO in some hardware is only because they need the IP sum zero'd during TSO, not because they need it for non-TSO packets....

Nov 5 2018, 11:05 PM

Nov 2 2018

jacob.e.keller_intel.com added a comment to D17801: TSO inherently requires checksum offload. In the case where TSO is in use, force the checksum offload on as well for that packet..

Thanks! This looks great.

Nov 2 2018, 9:17 PM
jacob.e.keller_intel.com added a comment to D17801: TSO inherently requires checksum offload. In the case where TSO is in use, force the checksum offload on as well for that packet..

So this is a step in the right direction, but isn't a complete fix for the issue. We also need to enable the ip_sum workaround in this case as well. Possibly we can just move that check to after the TX_OFFLOAD4 and TX_OFFLOAD6 checks?

Nov 2 2018, 7:43 PM
jacob.e.keller_intel.com added inline comments to D17801: TSO inherently requires checksum offload. In the case where TSO is in use, force the checksum offload on as well for that packet..
Nov 2 2018, 7:12 PM
jacob.e.keller_intel.com added a comment to D17801: TSO inherently requires checksum offload. In the case where TSO is in use, force the checksum offload on as well for that packet..

This doesn't seem like the right fix.

Nov 2 2018, 7:11 PM

May 11 2018

jacob.e.keller_intel.com updated the diff for D15343: iflib: mark irq allocation name parameter as constant.
  • taskqgroup: make tqg_name a constant
  • taskqgroup: mark name paramter of taskqgroup_attach as const
  • iflib: mark irq allocation name parameter as constant
May 11 2018, 6:50 PM
jacob.e.keller_intel.com added a comment to D15343: iflib: mark irq allocation name parameter as constant.

Looks like this involves a bit more cleanup to the taskqgroup code (since ultimately the name gets passed there). due to the scope I think I'll split it into two commits.

May 11 2018, 6:22 PM

May 8 2018

jacob.e.keller_intel.com added a comment to D15343: iflib: mark irq allocation name parameter as constant.

Hmmm... I think there were. I'll double check this later today and see what I missed. I have a separate system for sending the reviews because my test system doesn't have direct access, I probably just forgot a few lines..

May 8 2018, 5:28 PM

May 7 2018

jacob.e.keller_intel.com created D15343: iflib: mark irq allocation name parameter as constant.
May 7 2018, 11:15 PM

May 4 2018

jacob.e.keller_intel.com updated the summary of D15300: iflib: print message when iflib_tx_structures_setup fails.
May 4 2018, 11:33 PM
jacob.e.keller_intel.com updated the diff for D15300: iflib: print message when iflib_tx_structures_setup fails.

Remove the redundant message in iflib_device_register

May 4 2018, 11:32 PM
jacob.e.keller_intel.com abandoned D15304: iflib: print message when iflib_tx_structures_setup fails.

Oops didn't mean to upload a new revision..

May 4 2018, 11:31 PM
jacob.e.keller_intel.com created D15304: iflib: print message when iflib_tx_structures_setup fails.
May 4 2018, 11:30 PM
jacob.e.keller_intel.com added a comment to D15299: iflib: cleanup queues when iflib_device_register fail.

While we're cleaning this up, I'd either remove this print, or add a matching one for the tx setup, and remove the one in the caller. I realize that you can deduce which failed by how many prints you get, but that seems ... weird..

May 4 2018, 8:41 PM
jacob.e.keller_intel.com added a comment to D15300: iflib: print message when iflib_tx_structures_setup fails.

As mentioned on the other review, maybe remove the print in the caller, since it is now redundant..

May 4 2018, 8:40 PM
jacob.e.keller_intel.com created D15300: iflib: print message when iflib_tx_structures_setup fails.
May 4 2018, 6:53 PM
jacob.e.keller_intel.com created D15299: iflib: cleanup queues when iflib_device_register fail.
May 4 2018, 6:52 PM
jacob.e.keller_intel.com added a comment to D15285: iflib: fix invalid free during queue allocation failure.

Can we MFC this to stable/11? I originally found the bug on my FreeBSD 11.1 system.

May 4 2018, 6:30 PM