-
Notifications
You must be signed in to change notification settings - Fork 20
ON-14783: Use additional NIC flags for X3522 #68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v9_0
Are you sure you want to change the base?
Conversation
5c4025e
to
5fda5d2
Compare
/* For shared rxqs, we store the timestamp in a fake prefix when copying | ||
* from the shared rx buffer into our own packet buffer. */ | ||
if( sti_nic->flags & ZF_RES_NIC_FLAG_SHARED_RXQ ) | ||
st_nic->rx_prefix_len = ES_DZ_RX_PREFIX_SIZE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be based on EF_VI_CAP_RX_REF
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was based against Onload-9 branch which does not have this capability I can update it and rebase to master.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the following commit is merged to Onload-9 branch happy to update this: Xilinx-CNS/onload@7764b68
This commit adds two additional NIC flags for use with X3522 NICs. These being ZF_RES_NIC_FLAG_SHARED_RXQ and ZF_RES_NIC_FLAG_CTPIO_ONLY. These are then used to set up the stack.
pio_available = 0; | ||
zf_log_stack_warn(st, "Failed to query PIO capability (rc = %d)\n", rc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused in this case. You've set pio_available to 0, but we still try ef_pio_alloc
if !(attr->pio >= PIO_MUST_USE)
. I think we may as well exit here if ef_pio_alloc
will always fail for x3 - which it should right? That said, I see that you're trying to preserve previous behaviour so maybe you can keep it as is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes sorry, I pushed it but hadn't finished working on this. Either way I see two ways of proceeding with this:
- Remove the setting of pio_available as it is not used
- Change the code to use this new capability check rather than try allocating the pio buffer (hesitant to do so as although should provide the same end result uses different function calls)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind either option. If we always fail ef_pio_alloc
when the PIO capability is not supported, I would rather we not make the additional call. But it doesn't matter that we do, it's just more work.
Slightly related, this error message only seems to refer to the -ENOSYS
case - "the API does not know how to retrieve support for the supplied capability". I think this may be a tad misleading when reporting -EOPNOTSUPP
. You have reported the rc which would dispel some confusion.
Running with Warnings enabled on X3 you get output warning that the NIC is not running low-latency firmware.
This commit simply suppresses the warning on X3 NICs.
Without commit on X3522:
With commit on X3522:
With Commit on X2522 running full-feature firmware:
With commit on X2522 running ultra-low-latency firmware: