On 09/13/2017 09:37 AM, Shahaf Shuler wrote:
Introduce a new API to configure Rx offloads.

In the new API, offloads are divided into per-port and per-queue
offloads. The PMD reports capability for each of them.
Offloads are enabled using the existing DEV_RX_OFFLOAD_* flags.
To enable per-port offload, the offload should be set on both device
configuration and queue configuration. To enable per-queue offload, the
offloads can be set only on queue configuration.

Applications should set the ignore_offload_bitfield bit on rxmode
structure in order to move to the new API.

I think it would be useful to have the description in the documentation.
It is really important topic on how per-port and per-queue offloads coexist
and rules should be 100% clear for PMD maintainers and application
developers.

Please, also highlight how per-port and per-queue capabilities should be
advertised. I mean if per-queue capability should be reported as per-port
as well. I'd say no to avoid duplication of per-queue capabilities in two
places. If so, could you explain why to enable it should be specified in
both places. How should be treated configuration when the offload is
enabled on port, but disabled on queue level.

The old Rx offloads API is kept for the meanwhile, in order to enable a
smooth transition for PMDs and application to the new API.

Signed-off-by: Shahaf Shuler <shah...@mellanox.com>
---
  doc/guides/nics/features.rst  |  33 ++++----
  lib/librte_ether/rte_ethdev.c | 156 +++++++++++++++++++++++++++++++++----
  lib/librte_ether/rte_ethdev.h |  51 +++++++++++-
  3 files changed, 210 insertions(+), 30 deletions(-)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 37ffbc68c..4e68144ef 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -179,7 +179,7 @@ Jumbo frame
Supports Rx jumbo frames. -* **[uses] user config**: ``dev_conf.rxmode.jumbo_frame``,

May be it should be removed from documentation when it is removed from sources? I have no strong opinion, but it would be more clear to find it in the documentation
with its status specified (obsolete)
The note is applicable to all similar cases below.

[snip]

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 0adf3274a..ba7a2b2dc 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h

[snip]

@@ -907,6 +934,18 @@ struct rte_eth_conf {
  #define DEV_RX_OFFLOAD_QINQ_STRIP  0x00000020
  #define DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000040
  #define DEV_RX_OFFLOAD_MACSEC_STRIP     0x00000080
+#define DEV_RX_OFFLOAD_HEADER_SPLIT    0x00000100
+#define DEV_RX_OFFLOAD_VLAN_FILTER     0x00000200
+#define DEV_RX_OFFLOAD_VLAN_EXTEND     0x00000400
+#define DEV_RX_OFFLOAD_JUMBO_FRAME     0x00000800
+#define DEV_RX_OFFLOAD_CRC_STRIP       0x00001000
+#define DEV_RX_OFFLOAD_SCATTER         0x00002000
+#define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
+                                DEV_RX_OFFLOAD_UDP_CKSUM | \
+                                DEV_RX_OFFLOAD_TCP_CKSUM)
+#define DEV_RX_OFFLOAD_VLAN (DEV_RX_OFFLOAD_VLAN_STRIP | \
+                            DEV_RX_OFFLOAD_VLAN_FILTER | \
+                            DEV_RX_OFFLOAD_VLAN_EXTEND)

It is not directly related to the patch, but I'd like to highlight that Rx/Tx are asymmetric here
since SCTP is missing for Rx, but present for Tx.

[snip]

Reply via email to