diff options
author | Martin Braun <martin.braun@ettus.com> | 2021-11-02 11:25:56 +0100 |
---|---|---|
committer | Aaron Rossetto <aaron.rossetto@ni.com> | 2021-12-02 05:49:53 -0800 |
commit | 9f9c75ecd6e4a318dad5c378fbf260c8fd8c2922 (patch) | |
tree | c63b3439e6580e7f25ccaf142829bd5733d7f50a /host | |
parent | 38825f4e562b1eda1129b0f02f82c0bf4cf77276 (diff) | |
download | uhd-9f9c75ecd6e4a318dad5c378fbf260c8fd8c2922.tar.gz uhd-9f9c75ecd6e4a318dad5c378fbf260c8fd8c2922.tar.bz2 uhd-9f9c75ecd6e4a318dad5c378fbf260c8fd8c2922.zip |
rfnoc: Clarify usage of MTU vs. max payload size, remove DEFAULT_SPP
These two values where being mixed up in the code. To summarize:
- The MTU is the max CHDR packet size, including header & timestamp.
- The max payload is the total number of bytes regular payload plus
metadata that can be fit into into a CHDR packet. It is strictly
smaller than the MTU. For example, for 64-bit CHDR widths, if
a timestamp is desired, the max payload is 16 bytes smaller than
the MTU.
The other issue was that we were using a magic constant (DEFAULT_SPP)
which was causing conflicts with MTUs and max payloads.
This constant was harmful in multiple ways:
- The explanatory comment was incorrect (it stated it would cap packets
to 1500 bytes, which it didn't)
- It imposed random, hardcoded values that interfered with an 'spp
discovery', i.e., the ability to derive a good spp value from MTUs
- The current value capped packet sizes to 8000 bytes CHDR packets, even
when we wanted to use bigger ones
This patch changes the following:
- noc_block_base now has improved docs for MTU, and additional APIs
(get_max_payload_size(), get_chdr_hdr_len()) which return the
current payload size given MTU and CHDR width, and the CHDR header
length.
- The internally used graph nodes for TX and RX streamers also get
equipped with the same new two API calls.
- The radio, siggen, and replay block all where doing different
calculations for their spp/ipp values. Now, they all use the max
payload value to calculate spp/ipp. Unit tests where adapted
accordingly. Usage of DEFAULT_SPP was removed.
- The replay block used a hardcoded 16 bytes for header lengths, which
was replaced by get_chdr_hdr_len()
- The TX and RX streamers where discarding the MTU value and using the
max payload size as the MTU, which then propagated throughout the
graph. Now, both values are stored and can be used where appropriate.
Diffstat (limited to 'host')
21 files changed, 321 insertions, 96 deletions
diff --git a/host/include/uhd/rfnoc/defaults.hpp b/host/include/uhd/rfnoc/defaults.hpp index d0f6c2311..afc018810 100644 --- a/host/include/uhd/rfnoc/defaults.hpp +++ b/host/include/uhd/rfnoc/defaults.hpp @@ -44,9 +44,6 @@ static const std::string DEFAULT_BLOCK_NAME = "Block"; //! This NOC-ID is used to look up the default block static const uint32_t DEFAULT_NOC_ID = 0xFFFFFFFF; static const double DEFAULT_TICK_RATE = 1.0; -// Whenever we need a default spp value use this, unless there are some -// block/device-specific constraints. It will keep the frame size below 1500. -static const int DEFAULT_SPP = 1996; /*! The NoC ID is the unique identifier of the block type. All blocks of the * same type have the same NoC ID. diff --git a/host/include/uhd/rfnoc/noc_block_base.hpp b/host/include/uhd/rfnoc/noc_block_base.hpp index ab002841c..6b1ac5bcd 100644 --- a/host/include/uhd/rfnoc/noc_block_base.hpp +++ b/host/include/uhd/rfnoc/noc_block_base.hpp @@ -115,6 +115,10 @@ public: /*! Return the current MTU on a given edge * + * Note: The MTU is the maximum size of a CHDR packet, including header. In + * order to find out the maximum payload size, calling get_max_payload_size() + * is the recommended alternative. + * * The MTU is determined by the block itself (i.e., how big of a packet can * this block handle on this edge), but also the neighboring block, and * possibly the transport medium between the blocks. This value can thus be @@ -127,6 +131,51 @@ public: */ size_t get_mtu(const res_source_info& edge); + /*! Return the size of a CHDR packet header, in bytes. + * + * This helper function factors in the CHDR width for this block. + * + * \param account_for_ts If true (default), the assumption is that we reserve + * space for a timestamp. It is possible to increase + * the payload if no timestamp is used (only for 64 + * bit CHDR widths!), however, this is advanced usage + * and should only be used in special circumstances, + * as downstream blocks might not be able to handle + * such packets. + * \returns the length of a CHDR header in bytes + */ + size_t get_chdr_hdr_len(const bool account_for_ts = true) const; + + /*! Return the maximum usable payload size on a given edge, in bytes. + * + * This is very similar to get_mtu(), except it also accounts for the + * header. + * + * Example: Say the MTU on a given edge is 8192 bytes. The CHDR width is + * 64 bits. If we wanted to add a timestamp, we would thus require 16 bytes + * for the total header, leaving only 8192-16=8176 bytes for a payload, + * which is what this function would return. + * The same MTU, with a CHDR width of 512 bits however, would require leaving + * 64 bytes for the header (regardless of whether or not a timestamp is + * included). In that case, this function would return 8192-64=8128 bytes + * max payload size. + * + * \param edge The edge on which the max payload size is queried. edge.type + * must be INPUT_EDGE or OUTPUT_EDGE! See also get_mtu(). + * \param account_for_ts If true (default), the assumption is that we reserve + * space for a timestamp. It is possible to increase + * the payload if no timestamp is used (only for 64 + * bit CHDR widths!), however, this is advanced usage + * and should only be used in special circumstances, + * as downstream blocks might not be able to handle + * such packets. + * \returns the max payload size as determined by the overall graph on this + * edge, as well as the CHDR width. + * \throws uhd::value_error if edge is not referring to a valid edge + */ + size_t get_max_payload_size( + const res_source_info& edge, const bool account_for_ts = true); + /*! Return the arguments that were passed into this block from the framework */ uhd::device_addr_t get_block_args() const diff --git a/host/include/uhd/rfnoc/replay_block_control.hpp b/host/include/uhd/rfnoc/replay_block_control.hpp index dc56d59e0..c7624d044 100644 --- a/host/include/uhd/rfnoc/replay_block_control.hpp +++ b/host/include/uhd/rfnoc/replay_block_control.hpp @@ -297,7 +297,8 @@ public: /*! Set the maximum size of a packet * - * Sets the maximum packet size, inclusive of headers and payload. + * Sets the maximum packet size, inclusive of headers and payload. Cannot + * exceed the MTU for this block. * * \param size The size of the packet * \param port Which output port of the replay block to use diff --git a/host/lib/include/uhdlib/rfnoc/chdr_rx_data_xport.hpp b/host/lib/include/uhdlib/rfnoc/chdr_rx_data_xport.hpp index cb0987446..08930f9f3 100644 --- a/host/lib/include/uhdlib/rfnoc/chdr_rx_data_xport.hpp +++ b/host/lib/include/uhdlib/rfnoc/chdr_rx_data_xport.hpp @@ -177,13 +177,41 @@ public: */ ~chdr_rx_data_xport(); - /*! Returns maximum number payload bytes + /*! Returns MTU for this transport in bytes * - * \return maximum payload bytes per packet + * MTU is the max size for CHDR packets, including headers. For most + * applications, get_max_payload_size() is probably the more useful method. + * Compare also noc_block_base::get_mtu(). + * + * \return MTU in bytes + */ + size_t get_mtu() const + { + return _mtu; + } + + /*! Return the size of a CHDR packet header, in bytes. + * + * This helper function factors in the CHDR width for this transport. + * Compare also noc_block_base::get_chdr_hdr_len(). + * + * \returns the length of a CHDR header in bytes + */ + size_t get_chdr_hdr_len() const + { + return _hdr_len; + } + + /*! Returns maximum number of payload bytes + * + * This is smaller than the MTU. Compare also + * noc_block_base::get_max_payload_size(). + * + * \return maximum number of payload bytes */ size_t get_max_payload_size() const { - return _max_payload_size; + return _mtu - _hdr_len; } /*! @@ -389,8 +417,11 @@ private: // Flow control state rx_flow_ctrl_state _fc_state; - // Maximum data payload in bytes - size_t _max_payload_size = 0; + // MTU in bytes + size_t _mtu = 0; + + // Size of CHDR headers + size_t _hdr_len = 0; // Sequence number for data packets uint16_t _data_seq_num = 0; diff --git a/host/lib/include/uhdlib/rfnoc/chdr_tx_data_xport.hpp b/host/lib/include/uhdlib/rfnoc/chdr_tx_data_xport.hpp index 4e8ebe24d..e0887db6c 100644 --- a/host/lib/include/uhdlib/rfnoc/chdr_tx_data_xport.hpp +++ b/host/lib/include/uhdlib/rfnoc/chdr_tx_data_xport.hpp @@ -175,13 +175,41 @@ public: */ ~chdr_tx_data_xport(); + /*! Returns MTU for this transport in bytes + * + * MTU is the max size for CHDR packets, including headers. For most + * applications, get_max_payload_size() is probably the more useful method. + * Compare also noc_block_base::get_mtu(). + * + * \return MTU in bytes + */ + size_t get_mtu() const + { + return _mtu; + } + + /*! Return the size of a CHDR packet header, in bytes. + * + * This helper function factors in the CHDR width for this transport. + * Compare also noc_block_base::get_chdr_hdr_len(). + * + * \returns the length of a CHDR header in bytes + */ + size_t get_chdr_hdr_len() const + { + return _hdr_len; + } + /*! Returns maximum number of payload bytes * + * This is smaller than the MTU. Compare also + * noc_block_base::get_max_payload_size(). + * * \return maximum number of payload bytes */ size_t get_max_payload_size() const { - return _max_payload_size; + return _mtu - _hdr_len; } /*! @@ -373,8 +401,11 @@ private: // Flow control state tx_flow_ctrl_state _fc_state; - // Maximum data payload in bytes - size_t _max_payload_size = 0; + // MTU in bytes + size_t _mtu = 0; + + // Size of CHDR headers + size_t _hdr_len = 0; // Sequence number for data packets uint16_t _data_seq_num = 0; diff --git a/host/lib/include/uhdlib/rfnoc/radio_control_impl.hpp b/host/lib/include/uhdlib/rfnoc/radio_control_impl.hpp index b60b0f2a2..e268dbc07 100644 --- a/host/lib/include/uhdlib/rfnoc/radio_control_impl.hpp +++ b/host/lib/include/uhdlib/rfnoc/radio_control_impl.hpp @@ -356,6 +356,15 @@ private: const std::vector<uint32_t>& data, boost::optional<uint64_t> timestamp); + //! Return the maximum samples per packet of size \p bytes + // + // Given a packet of size \p bytes, how many samples can we fit in there? + // This gives the answer, factoring in item size and samples per clock. + // + // \param bytes Number of bytes we can fill with samples (excluding bytes + // required for CHDR headers!) + int get_max_spp(const size_t bytes); + //! FPGA compat number const uint32_t _fpga_compat; diff --git a/host/lib/include/uhdlib/transport/rx_streamer_impl.hpp b/host/lib/include/uhdlib/transport/rx_streamer_impl.hpp index 79c196f3f..f776d9373 100644 --- a/host/lib/include/uhdlib/transport/rx_streamer_impl.hpp +++ b/host/lib/include/uhdlib/transport/rx_streamer_impl.hpp @@ -88,7 +88,6 @@ public: if (stream_args.args.has_key("spp")) { _spp = stream_args.args.cast<size_t>("spp", _spp); - _mtu = _spp * _convert_info.bytes_per_otw_item; } } @@ -97,7 +96,8 @@ public: // them virtual void connect_channel(const size_t channel, typename transport_t::uptr xport) { - const size_t mtu = xport->get_max_payload_size(); + const size_t mtu = xport->get_mtu(); + _hdr_len = std::max(_hdr_len, xport->get_chdr_hdr_len()); _zero_copy_streamer.connect_channel(channel, std::move(xport)); if (mtu < _mtu) { @@ -201,11 +201,15 @@ protected: return _mtu; } - //! Sets the MTU and calculates spp + //! Sets the MTU and checks spp. If spp would exceed the new MTU, it is + // reduced accordingly. void set_mtu(const size_t mtu) { _mtu = mtu; - _spp = _mtu / _convert_info.bytes_per_otw_item; + const size_t spp_from_mtu = (_mtu - _hdr_len) / _convert_info.bytes_per_otw_item; + if (spp_from_mtu < _spp) { + _spp = spp_from_mtu; + } } //! Configures sample rate for conversion of timestamp @@ -401,7 +405,12 @@ private: // MTU, determined when xport is connected and modifiable by subclass size_t _mtu = std::numeric_limits<std::size_t>::max(); - // Maximum number of samples per packet + // Size of CHDR header in bytes + size_t _hdr_len = 0; + + // Maximum number of samples per packet. Note that this is not necessarily + // related to the MTU, it is a user-chosen value. However, it is always + // bounded by the MTU. size_t _spp = std::numeric_limits<std::size_t>::max(); // Num samps remaining in buffer currently held by zero copy streamer diff --git a/host/lib/include/uhdlib/transport/tx_streamer_impl.hpp b/host/lib/include/uhdlib/transport/tx_streamer_impl.hpp index f814c8192..9dc3b0c35 100644 --- a/host/lib/include/uhdlib/transport/tx_streamer_impl.hpp +++ b/host/lib/include/uhdlib/transport/tx_streamer_impl.hpp @@ -11,6 +11,7 @@ #include <uhd/stream.hpp> #include <uhd/types/metadata.hpp> #include <uhd/utils/tasks.hpp> +#include <uhd/utils/log.hpp> #include <uhdlib/transport/tx_streamer_zero_copy.hpp> #include <limits> #include <vector> @@ -111,13 +112,13 @@ public: if (stream_args.args.has_key("spp")) { _spp = stream_args.args.cast<size_t>("spp", _spp); - _mtu = _spp * _convert_info.bytes_per_otw_item; } } virtual void connect_channel(const size_t channel, typename transport_t::uptr xport) { - const size_t mtu = xport->get_max_payload_size(); + const size_t mtu = xport->get_mtu(); + _hdr_len = std::max(_hdr_len, xport->get_chdr_hdr_len()); _zero_copy_streamer.connect_channel(channel, std::move(xport)); if (mtu < _mtu) { @@ -323,11 +324,15 @@ protected: return _mtu; } - //! Sets the MTU and calculates spp + //! Sets the MTU and checks spp. If spp would exceed the new MTU, it is + // reduced accordingly. void set_mtu(const size_t mtu) { _mtu = mtu; - _spp = _mtu / _convert_info.bytes_per_otw_item; + const size_t spp_from_mtu = (_mtu - _hdr_len) / _convert_info.bytes_per_otw_item; + if (spp_from_mtu < _spp) { + _spp = spp_from_mtu; + } } //! Configures scaling factor for conversion @@ -444,7 +449,12 @@ private: // MTU, determined when xport is connected and modifiable by subclass size_t _mtu = std::numeric_limits<std::size_t>::max(); - // Maximum number of samples per packet + // Size of CHDR header in bytes + size_t _hdr_len = 0; + + // Maximum number of samples per packet. Note that this is not necessarily + // related to the MTU, it is a user-chosen value. However, it is always + // bounded by the MTU. size_t _spp = std::numeric_limits<std::size_t>::max(); // Metadata cache for send calls with no data diff --git a/host/lib/rfnoc/chdr_rx_data_xport.cpp b/host/lib/rfnoc/chdr_rx_data_xport.cpp index 7089c9071..37b3ffb89 100644 --- a/host/lib/rfnoc/chdr_rx_data_xport.cpp +++ b/host/lib/rfnoc/chdr_rx_data_xport.cpp @@ -5,6 +5,7 @@ // #include <uhd/rfnoc/chdr_types.hpp> +#include <uhd/utils/log.hpp> #include <uhdlib/rfnoc/chdr_rx_data_xport.hpp> #include <uhdlib/rfnoc/mgmt_portal.hpp> #include <uhdlib/rfnoc/rfnoc_common.hpp> @@ -39,6 +40,7 @@ chdr_rx_data_xport::chdr_rx_data_xport(uhd::transport::io_service::sptr io_srv, const fc_params_t& fc_params, disconnect_callback_t disconnect) : _fc_state(epids, fc_params.freq) + , _mtu(recv_link->get_recv_frame_size()) , _fc_sender(pkt_factory, epids) , _epid(epids.second) , _chdr_w_bytes(chdr_w_to_bits(pkt_factory.get_chdr_w()) / 8) @@ -52,10 +54,9 @@ chdr_rx_data_xport::chdr_rx_data_xport(uhd::transport::io_service::sptr io_srv, _recv_packet_cb = pkt_factory.make_generic(); _fc_sender.set_capacity(fc_params.buff_capacity); - // Calculate max payload size - const size_t pyld_offset = - _recv_packet->calculate_payload_offset(chdr::PKT_TYPE_DATA_WITH_TS); - _max_payload_size = recv_link->get_recv_frame_size() - pyld_offset; + // Calculate header size + _hdr_len = _recv_packet->calculate_payload_offset(chdr::PKT_TYPE_DATA_WITH_TS); + UHD_ASSERT_THROW(_hdr_len); // Make data transport auto recv_cb = diff --git a/host/lib/rfnoc/chdr_tx_data_xport.cpp b/host/lib/rfnoc/chdr_tx_data_xport.cpp index 7e926a163..618fffe5a 100644 --- a/host/lib/rfnoc/chdr_tx_data_xport.cpp +++ b/host/lib/rfnoc/chdr_tx_data_xport.cpp @@ -5,6 +5,7 @@ // #include <uhd/rfnoc/chdr_types.hpp> +#include <uhd/utils/log.hpp> #include <uhdlib/rfnoc/chdr_tx_data_xport.hpp> #include <uhdlib/rfnoc/mgmt_portal.hpp> #include <uhdlib/rfnoc/rfnoc_common.hpp> @@ -34,6 +35,7 @@ chdr_tx_data_xport::chdr_tx_data_xport(uhd::transport::io_service::sptr io_srv, const fc_params_t fc_params, disconnect_callback_t disconnect) : _fc_state(fc_params.buff_capacity) + , _mtu(send_link->get_send_frame_size()) , _fc_sender(pkt_factory, epids) , _epid(epids.first) , _chdr_w_bytes(chdr_w_to_bits(pkt_factory.get_chdr_w()) / 8) @@ -48,10 +50,9 @@ chdr_tx_data_xport::chdr_tx_data_xport(uhd::transport::io_service::sptr io_srv, _send_packet = pkt_factory.make_generic(); _recv_packet = pkt_factory.make_generic(); - // Calculate max payload size - const size_t pyld_offset = - _send_packet->calculate_payload_offset(chdr::PKT_TYPE_DATA_WITH_TS); - _max_payload_size = send_link->get_send_frame_size() - pyld_offset; + // Calculate header length + _hdr_len = _send_packet->calculate_payload_offset(chdr::PKT_TYPE_DATA_WITH_TS); + UHD_ASSERT_THROW(_hdr_len); // Now create the send I/O we will use for data auto send_cb = [this](buff_t::uptr buff, transport::send_link_if* send_link) { diff --git a/host/lib/rfnoc/noc_block_base.cpp b/host/lib/rfnoc/noc_block_base.cpp index c2db597cb..9719e739b 100644 --- a/host/lib/rfnoc/noc_block_base.cpp +++ b/host/lib/rfnoc/noc_block_base.cpp @@ -316,6 +316,21 @@ size_t noc_block_base::get_mtu(const res_source_info& edge) return _mtu.at(edge); } +size_t noc_block_base::get_chdr_hdr_len(const bool account_for_ts) const +{ + const size_t header_len_bytes = chdr_w_to_bits(_chdr_w) / 8; + // 64-bit CHDR requires two lines for the header if we use a timestamp, + // everything else requires one line + const size_t num_hdr_lines = (account_for_ts && _chdr_w == CHDR_W_64) ? 2 : 1; + return header_len_bytes * num_hdr_lines; +} + +size_t noc_block_base::get_max_payload_size( + const res_source_info& edge, const bool account_for_ts) +{ + return get_mtu(edge) - get_chdr_hdr_len(account_for_ts); +} + property_base_t* noc_block_base::get_mtu_prop_ref(const res_source_info& edge) { for (size_t mtu_prop_idx = 0; mtu_prop_idx < _mtu_props.size(); mtu_prop_idx++) { diff --git a/host/lib/rfnoc/radio_control_impl.cpp b/host/lib/rfnoc/radio_control_impl.cpp index 3c4c28d11..697bb2549 100644 --- a/host/lib/rfnoc/radio_control_impl.cpp +++ b/host/lib/rfnoc/radio_control_impl.cpp @@ -139,8 +139,11 @@ radio_control_impl::radio_control_impl(make_args_ptr make_args) _type_in.reserve(get_num_input_ports()); _type_out.reserve(get_num_output_ports()); for (size_t chan = 0; chan < get_num_output_ports(); ++chan) { + // Default SPP is the maximum value we can fit through the edge, given our MTU + const int default_spp = + get_max_spp(get_max_payload_size({res_source_info::OUTPUT_EDGE, chan})); _spp_prop.push_back( - property_t<int>(PROP_KEY_SPP, DEFAULT_SPP, {res_source_info::USER, chan})); + property_t<int>(PROP_KEY_SPP, default_spp, {res_source_info::USER, chan})); _samp_rate_in.push_back(property_t<double>( PROP_KEY_SAMP_RATE, get_tick_rate(), {res_source_info::INPUT_EDGE, chan})); _samp_rate_out.push_back(property_t<double>( @@ -166,17 +169,15 @@ radio_control_impl::radio_control_impl(make_args_ptr make_args) {&_spp_prop.back()}, [this, chan, &spp = _spp_prop.back()]() { RFNOC_LOG_TRACE("Calling resolver for spp@" << chan); - // MTU is max payload size, header with timestamp is already - // accounted for - const int mtu = - static_cast<int>(get_mtu({res_source_info::OUTPUT_EDGE, chan})); - const int mtu_samps = mtu / (_samp_width / 8); - const int max_spp_per_mtu = mtu_samps - (mtu_samps % _spc); - if (spp.get() > max_spp_per_mtu) { - RFNOC_LOG_DEBUG("spp value " << spp.get() << " exceeds MTU of " - << mtu << "! Coercing to " - << max_spp_per_mtu); - spp = max_spp_per_mtu; + const size_t max_pyld = + get_max_payload_size({res_source_info::OUTPUT_EDGE, chan}); + const int max_spp = get_max_spp(max_pyld); + if (spp.get() > max_spp) { + RFNOC_LOG_DEBUG("spp value " + << spp.get() << " exceeds MTU of " + << get_mtu({res_source_info::OUTPUT_EDGE, chan}) + << "! Coercing to " << max_spp); + spp = max_spp; } if (spp.get() % _spc) { spp = spp.get() - (spp.get() % _spc); @@ -185,7 +186,7 @@ radio_control_impl::radio_control_impl(make_args_ptr make_args) << spp.get()); } if (spp.get() <= 0) { - spp = DEFAULT_SPP; + spp = max_spp; RFNOC_LOG_WARNING( "spp must be greater than zero! Coercing to " << spp.get()); } @@ -198,7 +199,6 @@ radio_control_impl::radio_control_impl(make_args_ptr make_args) chan, &samp_rate_in = _samp_rate_in.at(chan), &samp_rate_out = _samp_rate_out.at(chan)]() { - const auto UHD_UNUSED(log_chan) = chan; RFNOC_LOG_TRACE("Calling resolver for samp_rate@" << chan); samp_rate_in = coerce_rate(samp_rate_in.get()); samp_rate_out = samp_rate_in.get(); @@ -1109,3 +1109,9 @@ void radio_control_impl::async_message_handler( boost::format("Received async message to invalid addr 0x%08X!") % addr)); } } + +int radio_control_impl::get_max_spp(const size_t bytes) +{ + const int packet_samps = bytes / (_samp_width / 8); + return packet_samps - (packet_samps % _spc); +} diff --git a/host/lib/rfnoc/replay_block_control.cpp b/host/lib/rfnoc/replay_block_control.cpp index e2d2523da..79e7446e4 100644 --- a/host/lib/rfnoc/replay_block_control.cpp +++ b/host/lib/rfnoc/replay_block_control.cpp @@ -124,7 +124,7 @@ public: _replay_reg_iface.poke64( REG_PLAY_BUFFER_SIZE_LO_ADDR, _play_size.at(port).get(), port); _replay_reg_iface.poke32(REG_PLAY_WORDS_PER_PKT_ADDR, - (_packet_size.at(port).get() - CHDR_MAX_LEN_HDR) / _word_size, + (_packet_size.at(port).get() - get_chdr_hdr_len()) / _word_size, port); } } @@ -226,7 +226,7 @@ public: uint32_t get_max_items_per_packet(const size_t port) const override { - return (_packet_size.at(port).get() - CHDR_MAX_LEN_HDR) + return (_packet_size.at(port).get() - get_chdr_hdr_len()) / get_play_item_size(port); } @@ -274,7 +274,7 @@ public: void set_max_items_per_packet(const uint32_t ipp, const size_t port) override { - set_max_packet_size(CHDR_MAX_LEN_HDR + ipp * get_play_item_size(port), port); + set_max_packet_size(get_chdr_hdr_len() + ipp * get_play_item_size(port), port); } void set_max_packet_size(const uint32_t size, const size_t port) override @@ -460,17 +460,26 @@ private: void _set_packet_size(const uint32_t packet_size, const size_t port) { - // MTU is max payload size, header with timestamp is already accounted for const size_t mtu = get_mtu({res_source_info::OUTPUT_EDGE, port}); - const uint32_t item_size = get_play_item_size(port); - const uint32_t mtu_payload = mtu - CHDR_MAX_LEN_HDR; - const uint32_t mtu_items = mtu_payload / item_size; - const uint32_t ipc = _word_size / item_size; // items per cycle - const uint32_t max_ipp_per_mtu = mtu_items - (mtu_items % ipc); - const uint32_t payload_size = packet_size - CHDR_MAX_LEN_HDR; - uint32_t ipp = payload_size / item_size; - if (ipp > max_ipp_per_mtu) { - ipp = max_ipp_per_mtu; + uint32_t requested_packet_size = packet_size; + if (requested_packet_size > mtu) { + requested_packet_size = mtu; + RFNOC_LOG_WARNING("Requested packet size exceeds MTU! Coercing to " + << requested_packet_size); + } + const size_t max_payload_bytes = + get_max_payload_size({res_source_info::OUTPUT_EDGE, port}); + const uint32_t item_size = get_play_item_size(port); + const uint32_t ipc = _word_size / item_size; // items per cycle + const uint32_t max_items = max_payload_bytes / item_size; + const uint32_t max_ipp = max_items - (max_items % ipc); + const uint32_t requested_payload_size = + requested_packet_size - (mtu - max_payload_bytes); + uint32_t ipp = requested_payload_size / item_size; + if (ipp > max_ipp) { + RFNOC_LOG_DEBUG("ipp value " << ipp << " exceeds MTU of " << mtu + << "! Coercing to " << max_ipp); + ipp = max_ipp; } if ((ipp % ipc) != 0) { ipp = ipp - (ipp % ipc); @@ -478,7 +487,7 @@ private: "ipp must be a multiple of the block bus width! Coercing to " << ipp); } if (ipp <= 0) { - ipp = DEFAULT_SPP; + ipp = max_ipp; RFNOC_LOG_WARNING("ipp must be greater than zero! Coercing to " << ipp); } // Packet size must be a multiple of word size diff --git a/host/lib/rfnoc/rfnoc_rx_streamer.cpp b/host/lib/rfnoc/rfnoc_rx_streamer.cpp index 7acdf357d..d57b8aab2 100644 --- a/host/lib/rfnoc/rfnoc_rx_streamer.cpp +++ b/host/lib/rfnoc/rfnoc_rx_streamer.cpp @@ -164,11 +164,15 @@ void rfnoc_rx_streamer::connect_channel( { UHD_ASSERT_THROW(channel < _mtu_in.size()); - // Update MTU property based on xport limits - const size_t mtu = xport->get_max_payload_size(); - set_property<size_t>(PROP_KEY_MTU, mtu, {res_source_info::INPUT_EDGE, channel}); + // Stash away the MTU before we lose access to xports + const size_t mtu = xport->get_mtu(); rx_streamer_impl<chdr_rx_data_xport>::connect_channel(channel, std::move(xport)); + + // Update MTU property based on xport limits. We need to do this after + // connect_channel(), because that's where the chdr_rx_data_xport object + // learns its header size. + set_property<size_t>(PROP_KEY_MTU, mtu, {res_source_info::INPUT_EDGE, channel}); } void rfnoc_rx_streamer::_register_props(const size_t chan, const std::string& otw_format) diff --git a/host/lib/rfnoc/rfnoc_tx_streamer.cpp b/host/lib/rfnoc/rfnoc_tx_streamer.cpp index b4aea202d..969c41ae6 100644 --- a/host/lib/rfnoc/rfnoc_tx_streamer.cpp +++ b/host/lib/rfnoc/rfnoc_tx_streamer.cpp @@ -125,9 +125,8 @@ void rfnoc_tx_streamer::connect_channel( { UHD_ASSERT_THROW(channel < _mtu_out.size()); - // Update MTU property based on xport limits - const size_t mtu = xport->get_max_payload_size(); - set_property<size_t>(PROP_KEY_MTU, mtu, {res_source_info::OUTPUT_EDGE, channel}); + // Stash away the MTU before we lose access to xports + const size_t mtu = xport->get_mtu(); xport->set_enqueue_async_msg_fn( [this, channel]( @@ -145,6 +144,11 @@ void rfnoc_tx_streamer::connect_channel( }); tx_streamer_impl<chdr_tx_data_xport>::connect_channel(channel, std::move(xport)); + + // Update MTU property based on xport limits. We need to do this after + // connect_channel(), because that's where the chdr_tx_data_xport object + // learns its header size. + set_property<size_t>(PROP_KEY_MTU, mtu, {res_source_info::OUTPUT_EDGE, channel}); } bool rfnoc_tx_streamer::recv_async_msg( diff --git a/host/lib/rfnoc/siggen_block_control.cpp b/host/lib/rfnoc/siggen_block_control.cpp index 2628d66ab..8dcc68319 100644 --- a/host/lib/rfnoc/siggen_block_control.cpp +++ b/host/lib/rfnoc/siggen_block_control.cpp @@ -154,9 +154,12 @@ private: PROP_KEY_CONSTANT_Q, 1.0, {res_source_info::USER, port}}); _prop_phase_inc.emplace_back(property_t<double>{ PROP_KEY_SINE_PHASE_INC, 1.0, {res_source_info::USER, port}}); + const int default_spp = + static_cast<int>( + get_max_payload_size({res_source_info::OUTPUT_EDGE, port})) + / uhd::convert::get_bytes_per_item(_prop_type_out.at(port).get()); _prop_spp.emplace_back(property_t<int>{ - PROP_KEY_SPP, DEFAULT_SPP, {res_source_info::USER, port}}); - + PROP_KEY_SPP, default_spp, {res_source_info::USER, port}}); register_property(&_prop_enable.back(), [this, port]() { _siggen_reg_iface.poke32(REG_ENABLE_OFFSET, uint32_t(_prop_enable.at(port).get() ? 1 : 0), @@ -256,21 +259,20 @@ private: get_mtu_prop_ref({res_source_info::OUTPUT_EDGE, port})}, {&_prop_spp.back()}, [this, port]() { - // MTU is max payload size, header with timestamp is already - // accounted for - int spp = _prop_spp.at(port).get(); - const int mtu = - static_cast<int>(get_mtu({res_source_info::OUTPUT_EDGE, port})); - const int mtu_samps = - mtu + int spp = _prop_spp.at(port).get(); + const int max_payload = static_cast<int>( + get_max_payload_size({res_source_info::OUTPUT_EDGE, port})); + const int max_samps = + max_payload / uhd::convert::get_bytes_per_item(_prop_type_out.at(port).get()); - if (spp > mtu_samps) { - RFNOC_LOG_WARNING("spp value " << spp << " exceeds MTU of " << mtu - << "! Coercing to " << mtu_samps); - spp = mtu_samps; + if (spp > max_samps) { + RFNOC_LOG_WARNING("spp value " << spp << " exceeds MTU of " + << max_payload << "! Coercing to " + << max_samps); + spp = max_samps; } if (spp <= 0) { - spp = DEFAULT_SPP; + spp = max_samps; RFNOC_LOG_WARNING( "spp must be greater than zero! Coercing to " << spp); } diff --git a/host/tests/rfnoc_block_tests/replay_block_test.cpp b/host/tests/rfnoc_block_tests/replay_block_test.cpp index f84b98ac8..db9f8c177 100644 --- a/host/tests/rfnoc_block_tests/replay_block_test.cpp +++ b/host/tests/rfnoc_block_tests/replay_block_test.cpp @@ -157,7 +157,7 @@ BOOST_FIXTURE_TEST_CASE(replay_test_construction, replay_block_fixture) BOOST_CHECK_EQUAL(reg_iface->write_memory[reg_play_base_addr], 0); BOOST_CHECK_EQUAL(reg_iface->write_memory[reg_play_base_addr + 4], 0); BOOST_CHECK_EQUAL(reg_iface->write_memory[reg_words_per_pkt], - (DEFAULT_MTU - CHDR_MAX_LEN_HDR) / word_size); + (DEFAULT_MTU - test_replay->get_chdr_hdr_len()) / word_size); BOOST_CHECK_EQUAL(reg_iface->write_memory[reg_play_item_size], default_item_size); } } @@ -353,18 +353,20 @@ BOOST_FIXTURE_TEST_CASE(replay_test_configure_play, replay_block_fixture) } /* - * This test case ensures that the hardware is programmed correctly throough the playback + * This test case ensures that the hardware is programmed correctly through the playback * packet API calls. */ BOOST_FIXTURE_TEST_CASE(replay_test_packet_size, replay_block_fixture) { for (size_t port = 0; port < num_output_ports; port++) { // Test the defaults - const uint32_t default_ipp = DEFAULT_SPP; - BOOST_CHECK_EQUAL(test_replay->get_max_items_per_packet(port), default_ipp); - - const uint32_t item_size = test_replay->get_play_item_size(port); - const uint32_t default_packet_size = default_ipp * item_size + CHDR_MAX_LEN_HDR; + const uint32_t item_size = test_replay->get_play_item_size(port); + const uint32_t expected_ipp = + test_replay->get_max_payload_size({res_source_info::OUTPUT_EDGE, port}) + / item_size; + BOOST_CHECK_EQUAL(test_replay->get_max_items_per_packet(port), expected_ipp); + const uint32_t default_packet_size = + expected_ipp * item_size + test_replay->get_chdr_hdr_len(); BOOST_CHECK_EQUAL(test_replay->get_max_packet_size(port), default_packet_size); } @@ -374,7 +376,7 @@ BOOST_FIXTURE_TEST_CASE(replay_test_packet_size, replay_block_fixture) BOOST_CHECK_EQUAL(test_replay->get_max_items_per_packet(port), ipp); const uint32_t item_size = test_replay->get_play_item_size(port); - const uint32_t packet_size = ipp * item_size + CHDR_MAX_LEN_HDR; + const uint32_t packet_size = ipp * item_size + test_replay->get_chdr_hdr_len(); BOOST_CHECK_EQUAL(test_replay->get_max_packet_size(port), packet_size); const uint32_t wpp = ipp * item_size / word_size; diff --git a/host/tests/rfnoc_block_tests/siggen_block_test.cpp b/host/tests/rfnoc_block_tests/siggen_block_test.cpp index ea52530e0..d453e898e 100644 --- a/host/tests/rfnoc_block_tests/siggen_block_test.cpp +++ b/host/tests/rfnoc_block_tests/siggen_block_test.cpp @@ -191,7 +191,10 @@ BOOST_FIXTURE_TEST_CASE(siggen_test_construction, siggen_block_fixture) BOOST_CHECK_EQUAL(reg_iface->phase_increments.at(port), siggen_mock_reg_iface_t::phase_increment_to_register(1.0)); BOOST_CHECK_EQUAL(reg_iface->phasors.at(port), siggen_mock_reg_iface_t::phasor_to_register({0.0, 0.0})); - BOOST_CHECK_EQUAL(reg_iface->spps.at(port), DEFAULT_SPP); + constexpr int bpi = 4; // sc16 has 4 bytes per item + const int expected_spp = + test_siggen->get_max_payload_size({res_source_info::OUTPUT_EDGE, port}) / bpi; + BOOST_CHECK_EQUAL(reg_iface->spps.at(port), expected_spp); } } @@ -315,10 +318,11 @@ BOOST_FIXTURE_TEST_CASE(siggen_test_ranges, siggen_block_fixture) */ BOOST_FIXTURE_TEST_CASE(siggen_test_spp_coercion, siggen_block_fixture) { - constexpr size_t mtu_in_samps = DEFAULT_MTU / 4 /* bytes/samp */; - size_t high_spp = mtu_in_samps + 10; - test_siggen->set_samples_per_packet(high_spp, 0); - BOOST_CHECK_EQUAL(test_siggen->get_samples_per_packet(0), mtu_in_samps); + const size_t max_samps = + (DEFAULT_MTU - test_siggen->get_chdr_hdr_len()) / 4 /* bytes/samp */; + const size_t too_high_spp = max_samps + 10; + test_siggen->set_samples_per_packet(too_high_spp, 0); + BOOST_CHECK_EQUAL(test_siggen->get_samples_per_packet(0), max_samps); } /* diff --git a/host/tests/rx_streamer_test.cpp b/host/tests/rx_streamer_test.cpp index 03ab3f456..a2969d608 100644 --- a/host/tests/rx_streamer_test.cpp +++ b/host/tests/rx_streamer_test.cpp @@ -95,9 +95,19 @@ public: _recv_link->release_recv_buff(std::move(buff)); } + size_t get_mtu() const + { + return _recv_link->get_recv_frame_size(); + } + + size_t get_chdr_hdr_len() const + { + return sizeof(mock_header_t); + } + size_t get_max_payload_size() const { - return _recv_link->get_recv_frame_size() - sizeof(packet_info_t); + return get_mtu() - get_chdr_hdr_len(); } private: diff --git a/host/tests/streamer_benchmark.cpp b/host/tests/streamer_benchmark.cpp index a11db3e68..b180e7393 100644 --- a/host/tests/streamer_benchmark.cpp +++ b/host/tests/streamer_benchmark.cpp @@ -76,11 +76,21 @@ public: _buff = std::move(buff); } - size_t get_max_payload_size() const + size_t get_mtu() const { return _buff_size; } + size_t get_chdr_hdr_len() const + { + return sizeof(packet_info_t); + } + + size_t get_max_payload_size() const + { + return get_mtu() - sizeof(packet_info_t); + } + private: size_t _buff_size; buff_t::uptr _buff; @@ -137,11 +147,22 @@ public: _buff = std::move(buff); } + size_t get_mtu() const + { + return _buff_size; + } + + size_t get_chdr_hdr_len() const + { + return sizeof(packet_info_t); + } + size_t get_max_payload_size() const { - return _buff_size - sizeof(packet_info_t); + return get_mtu() - sizeof(packet_info_t); } + private: size_t _buff_size; buff_t::uptr _buff; diff --git a/host/tests/tx_streamer_test.cpp b/host/tests/tx_streamer_test.cpp index 75511c329..11e33d413 100644 --- a/host/tests/tx_streamer_test.cpp +++ b/host/tests/tx_streamer_test.cpp @@ -53,10 +53,19 @@ public: _send_link->release_send_buff(std::move(buff)); } + size_t get_mtu() const + { + return _send_link->get_send_frame_size(); + } + + size_t get_chdr_hdr_len() const + { + return sizeof(packet_info_t); + } + size_t get_max_payload_size() const { - return _send_link->get_send_frame_size() - sizeof(packet_info_t); - ; + return get_mtu() - get_chdr_hdr_len(); } private: |