On 1/3/22 18:08, Akhil Goyal wrote:
IP Reassembly is a costly operation if it is done in software.
The operation becomes even more costlier if IP fragmants are encrypted.
However, if it is offloaded to HW, it can considerably save application cycles.
Hence, a new offload RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY is introduced in
ethdev for devices which can attempt reassembly of packets in hardware.
rte_eth_dev_info is updated with the reassembly capabilities which a device
can support.
Yes, reassembly is really complicated process taking possibility to have
overlapping fragments, out-of-order etc.
There are network attacks based on IP reassembly.
Will it simply result in IP reassembly failure if no buffers are left
for IP fragments? What will be reported in mbuf if some packets overlap?
Just raw packets as is or reassembly result with holes?
I think behavour should be specified/
The resulting reassembled packet would be a typical segmented mbuf in
case of success.
And if reassembly of fragments is failed or is incomplete (if fragments do
not come before the reass_timeout), the mbuf ol_flags can be updated.
This is updated in a subsequent patch.
Signed-off-by: Akhil Goyal <gak...@marvell.com>
---
doc/guides/nics/features.rst | 12 ++++++++++++
lib/ethdev/rte_ethdev.c | 1 +
lib/ethdev/rte_ethdev.h | 32 +++++++++++++++++++++++++++++++-
3 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 27be2d2576..1dfdee9602 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -602,6 +602,18 @@ Supports inner packet L4 checksum.
``tx_offload_capa,tx_queue_offload_capa:RTE_ETH_TX_OFFLOAD_OUTER_UDP_CKSUM``.
+.. _nic_features_ip_reassembly:
+
+IP reassembly
+-------------
+
+Supports IP reassembly in hardware.
+
+* **[uses] rte_eth_rxconf,rte_eth_rxmode**:
``offloads:RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY``.
Looking at the patch I see no changes and usage of rte_eth_rxconf and
rte_eth_rxmode. It should be added here later if corresponding changes
come in subsequent patches.
+* **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_IP_REASSEMBLY_INCOMPLETE``
Same here. The flag is not defined yet. So, it must not be mentioned in
the patch.
.
+* **[provides] rte_eth_dev_info**: ``reass_capa``.
+
+
.. _nic_features_shared_rx_queue:
Shared Rx queue
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index fa299c8ad7..11427b2e4d 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
[snip]
@@ -1781,6 +1782,33 @@ enum rte_eth_representor_type {
RTE_ETH_REPRESENTOR_PF, /**< representor of Physical Function. */
};
+/* Flag to offload IP reassembly for IPv4 packets. */
+#define RTE_ETH_DEV_REASSEMBLY_F_IPV4 (RTE_BIT32(0))
+/* Flag to offload IP reassembly for IPv6 packets. */
+#define RTE_ETH_DEV_REASSEMBLY_F_IPV6 (RTE_BIT32(1))
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice.
+ *
+ * A structure used to set IP reassembly configuration.
In the patch the structure is used to provide capabilities,
not to set configuration.
If you are going to use the same structure in capabilities and
configuration, it could be handy, but really confusing since
interpretation of fields should be different.
As a bare minimum the difference must be specified in comments.
Right now all fields makes sense in capabilities and configuration:
maximum possible vs actual value, however, not everything could be
really configurable and it will become confusing. It is really hard
to discuss right now since the patch does not provide usage of the
structure for the configuration.
+ *
+ * If RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY flag is set in offloads field,
+ * the PMD will attempt IP reassembly for the received packets as per
+ * properties defined in this structure:
+ *
+ */
+struct rte_eth_ip_reass_params {
+ /** Maximum time in ms which PMD can wait for other fragments. */
+ uint32_t reass_timeout;
Please, specify units. May be even in field name. E.g. reass_timeout_ms.
+ /** Maximum number of fragments that can be reassembled. */
+ uint16_t max_frags;
+ /**
+ * Flags to enable reassembly of packet types -
+ * RTE_ETH_DEV_REASSEMBLY_F_xxx.
+ */
+ uint16_t flags;
If it is just for packet types, I'd suggest to name the field more
precise. Also it will avoid flags vs frags misreading.
Just an idea. Up to you.
+};
+
/**
* A structure used to retrieve the contextual information of
* an Ethernet device, such as the controlling driver of the
@@ -1841,8 +1869,10 @@ struct rte_eth_dev_info {
* embedded managed interconnect/switch.
*/
struct rte_eth_switch_info switch_info;
+ /** IP reassembly offload capabilities that a device can support. */
+ struct rte_eth_ip_reass_params reass_capa;
- uint64_t reserved_64s[2]; /**< Reserved for future fields */
+ uint64_t reserved_64s[1]; /**< Reserved for future fields */
void *reserved_ptrs[2]; /**< Reserved for future fields */
};
