Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/include/zf_internal/private/zf_stack_def.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ struct zf_stack {
#define ZF_RES_NIC_FLAG_VLAN_FILTERS 0x1
#define ZF_RES_NIC_FLAG_RX_LL 0x2
#define ZF_RES_NIC_FLAG_TX_LL 0x4
#define ZF_RES_NIC_FLAG_SHARED_RXQ 0x8
#define ZF_RES_NIC_FLAG_CTPIO_ONLY 0x10

#include <onload/version.h>
#define ZF_VERSION_LENGTH_MAX OO_VER_STR_LEN
Expand Down
101 changes: 59 additions & 42 deletions src/lib/zf/private/stack_alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
\**************************************************************************/


#include <cerrno>
#include <zf/zf.h>
#include <zf_internal/private/zf_hal.h>
#include <zf_internal/zf_pool_res.h>
Expand Down Expand Up @@ -242,19 +243,25 @@ static int zf_stack_init_pio(struct zf_stack_impl* sti, struct zf_attr* attr,
struct zf_stack_nic* st_nic = &st->nic[nicno];
struct zf_stack_res_nic* sti_nic = &sti->nic[nicno];
ef_vi* vi = zf_stack_nic_tx_vi(st_nic);

if( st_nic->vi.nic_type.arch == EF_VI_ARCH_EFCT &&
attr->pio >= PIO_MUST_USE ) {
zf_log_stack_warn(st,
"PIO not supported by efct interface but pio=%d. "
"Not attempting to allocate PIO buffer.\n",
attr->pio);
st_nic->pio.busy = 3;
return 0;
unsigned long pio_available;
/* Check whether the NIC supports PIO mode. */
int rc = ef_pd_capabilities_get(sti_nic->dh, &sti_nic->pd, sti_nic->dh,
EF_VI_CAP_PIO, &pio_available);
if ( rc != 0 ) {
pio_available = 0;
zf_log_stack_warn(st, "Failed to query PIO capability (rc = %d)\n", rc);
Comment on lines +251 to +252
Copy link
Contributor

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

Copy link
Contributor Author

@ibeecraft-amd ibeecraft-amd Apr 2, 2025

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:

  1. Remove the setting of pio_available as it is not used
  2. 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)

Copy link
Contributor

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.

if( attr->pio >= PIO_MUST_USE ) {
zf_log_stack_warn(st,
"PIO not supported by %s but pio=%d. "
"Not attempting to allocate PIO buffer.\n",
sti_nic->if_name, attr->pio);
st_nic->pio.busy = 3;
return 0;
}
}

/* Try to allocate. If alloc fails and we are in-strict mode (2) then fail */
int rc = ef_pio_alloc(&sti_nic->pio, sti_nic->dh, &sti_nic->pd,
rc = ef_pio_alloc(&sti_nic->pio, sti_nic->dh, &sti_nic->pd,
-1, sti_nic->dh);
if( rc < 0 && attr->pio >= PIO_MUST_USE ) {
zf_log_stack_err(st, "Failed to allocate PIO buffer (rc = %d)\n", rc);
Expand Down Expand Up @@ -499,36 +506,45 @@ static int zf_stack_init_nic_capabilities(struct zf_stack* st, int nicno)

sti->nic[nicno].flags = 0;

rc = ef_pd_capabilities_get(dh, pd, dh, EF_VI_CAP_RX_FW_VARIANT, &variant);
if( rc != 0 ) {
zf_log_stack_err(st, "Failed to query RX mode for interface %s (rc %d)\n",
sti->nic[nicno].if_name, rc);
return rc;
}
else if( variant != MC_CMD_GET_CAPABILITIES_OUT_RXDP_LOW_LATENCY &&
variant != MC_CMD_GET_CAPABILITIES_OUT_RXDP ) {
zf_log_stack_err(st, "Interface %s is not in supported mode for RX (%d).\n",
sti->nic[nicno].if_name, variant);
return -EOPNOTSUPP;
rc = ef_pd_capabilities_get(dh, pd, dh, EF_VI_CAP_RX_SHARED, &variant);
if( rc == 0 )
sti->nic[nicno].flags |= ZF_RES_NIC_FLAG_SHARED_RXQ;
else {
rc = ef_pd_capabilities_get(dh, pd, dh, EF_VI_CAP_RX_FW_VARIANT, &variant);
if ( rc != 0 ) {
zf_log_stack_err(st, "Failed to query RX mode for interface %s (rc %d)\n",
sti->nic[nicno].if_name, rc);
return rc;
}
else if( variant != MC_CMD_GET_CAPABILITIES_OUT_RXDP_LOW_LATENCY &&
variant != MC_CMD_GET_CAPABILITIES_OUT_RXDP ) {
zf_log_stack_err(st, "Interface %s is not in supported mode for RX (%d).\n",
sti->nic[nicno].if_name, variant);
return -EOPNOTSUPP;
}
if( variant == MC_CMD_GET_CAPABILITIES_OUT_RXDP_LOW_LATENCY )
sti->nic[nicno].flags |= ZF_RES_NIC_FLAG_RX_LL;
}
if( variant == MC_CMD_GET_CAPABILITIES_OUT_RXDP_LOW_LATENCY )
sti->nic[nicno].flags |= ZF_RES_NIC_FLAG_RX_LL;

rc = ef_pd_capabilities_get(dh, pd, dh, EF_VI_CAP_TX_FW_VARIANT, &variant);
if( rc != 0 ) {
zf_log_stack_err(st, "Failed to query TX mode for interface %s (rc %d)\n",
sti->nic[nicno].if_name, rc);
return rc;
}
else if( variant != MC_CMD_GET_CAPABILITIES_OUT_TXDP_LOW_LATENCY &&
variant != MC_CMD_GET_CAPABILITIES_OUT_TXDP ) {
zf_log_stack_err(st, "Interface %s is not in supported mode for TX (%d).\n",
sti->nic[nicno].if_name, variant);
return -EOPNOTSUPP;
rc = ef_pd_capabilities_get(dh, pd, dh, EF_VI_CAP_CTPIO_ONLY, &variant);
if( rc == 0 )
sti->nic[nicno].flags |= ZF_RES_NIC_FLAG_CTPIO_ONLY;
else {
rc = ef_pd_capabilities_get(dh, pd, dh, EF_VI_CAP_TX_FW_VARIANT, &variant);
if( rc != 0 ) {
zf_log_stack_err(st, "Failed to query TX mode for interface %s (rc %d)\n",
sti->nic[nicno].if_name, rc);
return rc;
}
else if( variant != MC_CMD_GET_CAPABILITIES_OUT_TXDP_LOW_LATENCY &&
variant != MC_CMD_GET_CAPABILITIES_OUT_TXDP ) {
zf_log_stack_err(st, "Interface %s is not in supported mode for TX (%d).\n",
sti->nic[nicno].if_name, variant);
return -EOPNOTSUPP;
}
if( variant == MC_CMD_GET_CAPABILITIES_OUT_TXDP_LOW_LATENCY )
sti->nic[nicno].flags |= ZF_RES_NIC_FLAG_TX_LL;
}
if( variant == MC_CMD_GET_CAPABILITIES_OUT_TXDP_LOW_LATENCY )
sti->nic[nicno].flags |= ZF_RES_NIC_FLAG_TX_LL;

rc = ef_pd_capabilities_get(dh, pd, dh, EF_VI_CAP_RX_FILTER_TYPE_IP_VLAN,
&vlan_filters);
if( rc == 0 && vlan_filters != 0 )
Expand Down Expand Up @@ -574,8 +590,9 @@ int zf_stack_init_nic_resources(struct zf_stack_impl* sti,
if( rc < 0 )
goto fail1;

if( !(sti->nic[nicno].flags & ZF_RES_NIC_FLAG_RX_LL) ||
!(sti->nic[nicno].flags & ZF_RES_NIC_FLAG_TX_LL) ) {
if( (!(sti_nic->flags & ZF_RES_NIC_FLAG_RX_LL) ||
!(sti_nic->flags & ZF_RES_NIC_FLAG_TX_LL)) &&
!(sti_nic->flags & ZF_RES_NIC_FLAG_SHARED_RXQ) ) {
zf_log_stack_warn(st, "Interface %s is not in low latency mode.\n",
sti->nic[nicno].if_name);
zf_log_stack_warn(st, "Low latency mode is recommended for best "
Expand Down Expand Up @@ -636,9 +653,9 @@ int zf_stack_init_nic_resources(struct zf_stack_impl* sti,
);

if ( attr->rx_ring_max != 0 ) {
/* For EFCT, we store the timestamp in a fake prefix when copying from
* the shared rx buffer into our own packet buffer. */
if( st_nic->vi.nic_type.arch == EF_VI_ARCH_EFCT )
/* 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;
Comment on lines +656 to 659
Copy link
Collaborator

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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

else
st_nic->rx_prefix_len = ef_vi_receive_prefix_len(&st_nic->vi);
Expand Down