| Message ID | 20230302203045.4101652-5-shayagr@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add tx push buf len param to ethtool | expand |
On Thu, Mar 02, 2023 at 10:30:45PM +0200, Shay Agroskin wrote: > The ENA driver allows for two distinct values for the number of bytes > of the packet's payload that can be written directly to the device. > > For a value of 224 the driver turns on Large LLQ Header mode in which > the first 224 of the packet's payload are written to the LLQ. > > Signed-off-by: Shay Agroskin <shayagr@amazon.com> Reviewed-by: Simon Horman <simon.horman@corigine.com>
On Thu, 2 Mar 2023 22:30:45 +0200 Shay Agroskin wrote: > @@ -496,11 +509,40 @@ static int ena_set_ringparam(struct net_device *netdev, > ENA_MIN_RING_SIZE : ring->rx_pending; > new_rx_size = rounddown_pow_of_two(new_rx_size); > > - if (new_tx_size == adapter->requested_tx_ring_size && > - new_rx_size == adapter->requested_rx_ring_size) > + changed |= new_tx_size != adapter->requested_tx_ring_size || > + new_rx_size != adapter->requested_rx_ring_size; > + > + /* This value is ignored if LLQ is not supported */ > + new_tx_push_buf_len = 0; > + if (adapter->ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_HOST) > + goto no_llq_supported; Are you rejecting the unsupported config in this case or just ignoring it? You need to return an error if user tries to set something the device does not support/allow. BTW your use of gotos to skip code is against the kernel coding style. gotos are only for complex cases and error handling, you're using them to save indentation it seems. Factor the code out to a helper instead, or some such. > + new_tx_push_buf_len = kernel_ring->tx_push_buf_len; > + > + /* support for ENA_LLQ_LARGE_HEADER is tested in the 'get' command */ > + if (new_tx_push_buf_len != ENA_LLQ_HEADER && > + new_tx_push_buf_len != ENA_LLQ_LARGE_HEADER) { > + bool large_llq_sup = adapter->large_llq_header_supported; > + char large_llq_size_str[40]; > + > + snprintf(large_llq_size_str, 40, ", %lu", ENA_LLQ_LARGE_HEADER); > + > + NL_SET_ERR_MSG_FMT_MOD(extack, > + "Only [%lu%s] tx push buff length values are supported", > + ENA_LLQ_HEADER, > + large_llq_sup ? large_llq_size_str : ""); > + > + return -EINVAL; > + } > + > + changed |= new_tx_push_buf_len != adapter->ena_dev->tx_max_header_size; > + > +no_llq_supported: > + if (!changed) > return 0; > > - return ena_update_queue_sizes(adapter, new_tx_size, new_rx_size); > + return ena_update_queue_params(adapter, new_tx_size, new_rx_size, > + new_tx_push_buf_len);
Jakub Kicinski <kuba@kernel.org> writes: > On Thu, 2 Mar 2023 22:30:45 +0200 Shay Agroskin wrote: >> @@ -496,11 +509,40 @@ static int ena_set_ringparam(struct >> net_device *netdev, >> ENA_MIN_RING_SIZE : ring->rx_pending; >> new_rx_size = rounddown_pow_of_two(new_rx_size); >> >> - if (new_tx_size == adapter->requested_tx_ring_size && >> - new_rx_size == adapter->requested_rx_ring_size) >> + changed |= new_tx_size != adapter->requested_tx_ring_size >> || >> + new_rx_size != adapter->requested_rx_ring_size; >> + >> + /* This value is ignored if LLQ is not supported */ >> + new_tx_push_buf_len = 0; >> + if (adapter->ena_dev->tx_mem_queue_type == >> ENA_ADMIN_PLACEMENT_POLICY_HOST) >> + goto no_llq_supported; > > Are you rejecting the unsupported config in this case or just > ignoring > it? You need to return an error if user tries to set something > the > device does not support/allow. > I'll explicitly set 0s to push buffer in 'get' when LLQ isn't supported and return -ENOSUPP if the user tries to set it when no LLQ is used. > BTW your use of gotos to skip code is against the kernel coding > style. > gotos are only for complex cases and error handling, you're > using them > to save indentation it seems. Factor the code out to a helper > instead, > or some such. > Modified the code to remove the gotos (although I thought they were an elegant implementation) >> + new_tx_push_buf_len = kernel_ring->tx_push_buf_len; >> + >> + /* support for ENA_LLQ_LARGE_HEADER is tested in the 'get' >> command */ >> + if (new_tx_push_buf_len != ENA_LLQ_HEADER && >> + new_tx_push_buf_len != ENA_LLQ_LARGE_HEADER) { >> + bool large_llq_sup = >> adapter->large_llq_header_supported; >> + char large_llq_size_str[40]; >> + >> + snprintf(large_llq_size_str, 40, ", %lu", >> ENA_LLQ_LARGE_HEADER); >> + >> + NL_SET_ERR_MSG_FMT_MOD(extack, >> + "Only [%lu%s] tx push buff >> length values are supported", >> + ENA_LLQ_HEADER, >> + large_llq_sup ? >> large_llq_size_str : ""); >> + >> + return -EINVAL; >> + } >> + >> + changed |= new_tx_push_buf_len != >> adapter->ena_dev->tx_max_header_size; >> + >> +no_llq_supported: >> + if (!changed) >> return 0; >> >> - return ena_update_queue_sizes(adapter, new_tx_size, >> new_rx_size); >> + return ena_update_queue_params(adapter, new_tx_size, >> new_rx_size, >> + new_tx_push_buf_len);
diff --git a/drivers/net/ethernet/amazon/ena/ena_eth_com.h b/drivers/net/ethernet/amazon/ena/ena_eth_com.h index 689313ee25a8..8bec31fa816c 100644 --- a/drivers/net/ethernet/amazon/ena/ena_eth_com.h +++ b/drivers/net/ethernet/amazon/ena/ena_eth_com.h @@ -10,6 +10,10 @@ /* head update threshold in units of (queue size / ENA_COMP_HEAD_THRESH) */ #define ENA_COMP_HEAD_THRESH 4 +/* we allow 2 DMA descriptors per LLQ entry */ +#define ENA_LLQ_ENTRY_DESC_CHUNK_SIZE (2 * sizeof(struct ena_eth_io_tx_desc)) +#define ENA_LLQ_HEADER (128 - ENA_LLQ_ENTRY_DESC_CHUNK_SIZE) +#define ENA_LLQ_LARGE_HEADER (256 - ENA_LLQ_ENTRY_DESC_CHUNK_SIZE) struct ena_com_tx_ctx { struct ena_com_tx_meta ena_meta; diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c index 8da79eedc057..a692e6c907ae 100644 --- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c +++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c @@ -476,6 +476,18 @@ static void ena_get_ringparam(struct net_device *netdev, ring->tx_max_pending = adapter->max_tx_ring_size; ring->rx_max_pending = adapter->max_rx_ring_size; + if (adapter->ena_dev->tx_mem_queue_type == + ENA_ADMIN_PLACEMENT_POLICY_HOST) + goto no_llq_supported; + + if (adapter->large_llq_header_supported) + kernel_ring->tx_push_buf_max_len = ENA_LLQ_LARGE_HEADER; + else + kernel_ring->tx_push_buf_max_len = ENA_LLQ_HEADER; + + kernel_ring->tx_push_buf_len = adapter->ena_dev->tx_max_header_size; + +no_llq_supported: ring->tx_pending = adapter->tx_ring[0].ring_size; ring->rx_pending = adapter->rx_ring[0].ring_size; } @@ -486,7 +498,8 @@ static int ena_set_ringparam(struct net_device *netdev, struct netlink_ext_ack *extack) { struct ena_adapter *adapter = netdev_priv(netdev); - u32 new_tx_size, new_rx_size; + u32 new_tx_size, new_rx_size, new_tx_push_buf_len; + bool changed = false; new_tx_size = ring->tx_pending < ENA_MIN_RING_SIZE ? ENA_MIN_RING_SIZE : ring->tx_pending; @@ -496,11 +509,40 @@ static int ena_set_ringparam(struct net_device *netdev, ENA_MIN_RING_SIZE : ring->rx_pending; new_rx_size = rounddown_pow_of_two(new_rx_size); - if (new_tx_size == adapter->requested_tx_ring_size && - new_rx_size == adapter->requested_rx_ring_size) + changed |= new_tx_size != adapter->requested_tx_ring_size || + new_rx_size != adapter->requested_rx_ring_size; + + /* This value is ignored if LLQ is not supported */ + new_tx_push_buf_len = 0; + if (adapter->ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_HOST) + goto no_llq_supported; + + new_tx_push_buf_len = kernel_ring->tx_push_buf_len; + + /* support for ENA_LLQ_LARGE_HEADER is tested in the 'get' command */ + if (new_tx_push_buf_len != ENA_LLQ_HEADER && + new_tx_push_buf_len != ENA_LLQ_LARGE_HEADER) { + bool large_llq_sup = adapter->large_llq_header_supported; + char large_llq_size_str[40]; + + snprintf(large_llq_size_str, 40, ", %lu", ENA_LLQ_LARGE_HEADER); + + NL_SET_ERR_MSG_FMT_MOD(extack, + "Only [%lu%s] tx push buff length values are supported", + ENA_LLQ_HEADER, + large_llq_sup ? large_llq_size_str : ""); + + return -EINVAL; + } + + changed |= new_tx_push_buf_len != adapter->ena_dev->tx_max_header_size; + +no_llq_supported: + if (!changed) return 0; - return ena_update_queue_sizes(adapter, new_tx_size, new_rx_size); + return ena_update_queue_params(adapter, new_tx_size, new_rx_size, + new_tx_push_buf_len); } static u32 ena_flow_hash_to_flow_type(u16 hash_fields) @@ -900,6 +942,7 @@ static int ena_set_tunable(struct net_device *netdev, static const struct ethtool_ops ena_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_USECS | ETHTOOL_COALESCE_USE_ADAPTIVE_RX, + .supported_ring_params = ETHTOOL_RING_USE_TX_PUSH_BUF_LEN, .get_link_ksettings = ena_get_link_ksettings, .get_drvinfo = ena_get_drvinfo, .get_msglevel = ena_get_msglevel, diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c index 43e3c76bd6ae..0625be4619a8 100644 --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c @@ -2811,11 +2811,13 @@ static int ena_close(struct net_device *netdev) return 0; } -int ena_update_queue_sizes(struct ena_adapter *adapter, - u32 new_tx_size, - u32 new_rx_size) +int ena_update_queue_params(struct ena_adapter *adapter, + u32 new_tx_size, + u32 new_rx_size, + u32 new_llq_header_len) { - bool dev_was_up; + bool dev_was_up, large_llq_changed = false; + int rc = 0; dev_was_up = test_bit(ENA_FLAG_DEV_UP, &adapter->flags); ena_close(adapter->netdev); @@ -2825,7 +2827,23 @@ int ena_update_queue_sizes(struct ena_adapter *adapter, 0, adapter->xdp_num_queues + adapter->num_io_queues); - return dev_was_up ? ena_up(adapter) : 0; + + large_llq_changed = adapter->ena_dev->tx_mem_queue_type == + ENA_ADMIN_PLACEMENT_POLICY_DEV; + large_llq_changed &= + new_llq_header_len != adapter->ena_dev->tx_max_header_size; + + /* a check that the configuration is valid is done by caller */ + if (!large_llq_changed) + goto if_up; + + adapter->large_llq_header_enabled = !adapter->large_llq_header_enabled; + + ena_destroy_device(adapter, false); + rc = ena_restore_device(adapter); + +if_up: + return dev_was_up && !rc ? ena_up(adapter) : 0; } int ena_set_rx_copybreak(struct ena_adapter *adapter, u32 rx_copybreak) diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h index 3e8c4a66c7d8..5a0d4ee76172 100644 --- a/drivers/net/ethernet/amazon/ena/ena_netdev.h +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h @@ -396,9 +396,10 @@ void ena_dump_stats_to_buf(struct ena_adapter *adapter, u8 *buf); int ena_update_hw_stats(struct ena_adapter *adapter); -int ena_update_queue_sizes(struct ena_adapter *adapter, - u32 new_tx_size, - u32 new_rx_size); +int ena_update_queue_params(struct ena_adapter *adapter, + u32 new_tx_size, + u32 new_rx_size, + u32 new_llq_header_len); int ena_update_queue_count(struct ena_adapter *adapter, u32 new_channel_count);
The ENA driver allows for two distinct values for the number of bytes of the packet's payload that can be written directly to the device. For a value of 224 the driver turns on Large LLQ Header mode in which the first 224 of the packet's payload are written to the LLQ. Signed-off-by: Shay Agroskin <shayagr@amazon.com> --- drivers/net/ethernet/amazon/ena/ena_eth_com.h | 4 ++ drivers/net/ethernet/amazon/ena/ena_ethtool.c | 51 +++++++++++++++++-- drivers/net/ethernet/amazon/ena/ena_netdev.c | 28 ++++++++-- drivers/net/ethernet/amazon/ena/ena_netdev.h | 7 +-- 4 files changed, 78 insertions(+), 12 deletions(-)