issue: 3613619 Avoid posting RX WQEs for Neigh ring #78
Conversation
AlexanderGrissik
force-pushed
the
tx_only_ring
branch
from
a5e4106
to
1a1ec20
Compare
|
I called this RING_ETH_TX. Let's discuss later if we want to change it to RING_ETH_NEIGH.. |
AlexanderGrissik
force-pushed
the
tx_only_ring
branch
from
1a1ec20
to
5a67a4d
Compare
src/core/util/xlio_stats.h
Outdated
| @@ -351,9 +351,9 @@ typedef struct { | |||
| cq_stats_t cq_stats; | |||
| } cq_instance_block_t; | |||
|
|
|||
| typedef enum { RING_ETH = 0, RING_TAP } ring_type_t; | |||
| typedef enum { RING_ETH = 0, RING_TAP, RING_ETH_TX } ring_type_t; | |||
There was a problem hiding this comment.
order ETH, ETH_TX, TAP looks more readable (if you change order here, change it for the strings array too)
| @@ -271,7 +271,7 @@ typedef enum { | |||
| RING_LOGIC_PER_THREAD = 20, //!< RING_LOGIC_PER_THREAD | |||
| RING_LOGIC_PER_CORE = 30, //!< RING_LOGIC_PER_CORE | |||
| RING_LOGIC_PER_CORE_ATTACH_THREADS = 31, //!< RING_LOGIC_PER_CORE_ATTACH_THREADS | |||
| RING_LOGIC_PER_OBJECT = 32, //!< RING_LOGIC_PER_OBJECT | |||
| RING_LOGIC_NEIGH = 32, //!< RING_LOGIC_NEIGH | |||
| RING_LOGIC_ISOLATE = 33, //!< RING_LOGIC_ISOLATE | |||
| RING_LOGIC_LAST //!< RING_LOGIC_LAST | |||
There was a problem hiding this comment.
[suggestion]
xlio_extra.h:
enum {
...
RING_LOGIC_PER_CORE_ATTACH_THREADS = 31,
RING_LOGIC_LAST = 100
};ring_allocation_logic.h:
/* Internal ring allocation logic which is hidden from user. */
enum {
RING_LOGIC_NEIGH = RING_LOGIC_LAST + 1,
RING_LOGIC_ISOLATE,
};ring_allocation_logic.cpp:
bool is_logic_support_migration()
{
...
m_res_key.get_ring_alloc_logic() < **RING_LOGIC_LAST**
}There was a problem hiding this comment.
I am not sure about this change. It will create confusing conversions between ring_logic_t and ring_logic_public_t.
If we have to hide internal logics from the user of xlio_extra maybe it is better:
move ring_logic_t to ring_allocation_logic.h and make
enum {
...
RING_LOGIC_PER_CORE_ATTACH_THREADS = 31
} ring_logic_public_t
just for extra_api.h usage. The negative of this approach is that we have kind of duplicate definitions.
The best would be deriving one enum from another, but this is not possible in CPP.
There was a problem hiding this comment.
this is extra API and will be replaced by storage api with the groups. Depending on further steps, we may even remove all the obsolete extra API. So lets keep it as is and improve once we do cleanup.
Neigh ring is used for TX only, to send ARP/ICMPv6 packets. However each XLIO ring creates both SQ and RQ. By avoiding posting RX WQEs to Neigh ring RQ, a considerable amount of memory is saved. Signed-off-by: Alexander Grissik <agrissik@nvidia.com>
AlexanderGrissik
force-pushed
the
tx_only_ring
branch
from
5a67a4d
to
bc77ea7
Compare
|
I rebased on top of vNext. |
| @@ -61,11 +61,10 @@ class ring_alloc_logic_attr { | |||
| ring_alloc_logic_attr(const ring_alloc_logic_attr &other); | |||
| void set_ring_alloc_logic(ring_logic_t logic); | |||
| void set_user_id_key(uint64_t user_id_key); | |||
| void set_use_locks(bool use_locks); | |||
There was a problem hiding this comment.
Don't remove it, new storage API uses it.
| : m_p_ring(ring) | ||
| , m_p_ib_ctx_handler(ib_ctx) | ||
| , m_n_sysvar_rx_num_wr_to_post_recv(safe_mce_sys().rx_num_wr_to_post_recv) | ||
| , m_rx_num_wr(align32pow2(safe_mce_sys().rx_num_wr)) | ||
| , m_n_sysvar_rx_prefetch_bytes_before_poll(safe_mce_sys().rx_prefetch_bytes_before_poll) | ||
| , m_vlan(vlan) | ||
| , m_dummy(dummy) |
There was a problem hiding this comment.
Would be better to make decision on dummy attribute in the constructor instead of storing it (lets say pass the information about dummy or number of buffers to confiure_rq()). If the RQ is really dummy, it won't be used and won't trigger any methods to post new buffers to the RQ.
If this cannot be done in context of constructor, then leave it in current state.
Description
Neigh ring was used only for TX and wasted RX buffers.
What
For Neigh ring we mark this ring as TX only and when QP is created RX WQEs are not posted to it's RQ.
This allows saving memory especially for multi process use cases.
Why ?
Avoid waste of memory.
Change type
What kind of change does this PR introduce?
Check list