[PATCH 0/4] add support of partial offload
This patch series aims to add support of partial offload for NFP PMD. Long Wu (4): net/nfp: support MARK flow action net/nfp: add interface to check representor net/nfp: representor adds RSS configuration net/nfp: support RSS flow action doc/guides/nics/features/nfp.ini | 1 + drivers/common/nfp/nfp_common_ctrl.h | 1 + drivers/net/nfp/flower/nfp_flower.c | 12 +- drivers/net/nfp/flower/nfp_flower_cmsg.h | 16 ++ drivers/net/nfp/flower/nfp_flower_flow.c | 153 +- drivers/net/nfp/flower/nfp_flower_flow.h | 8 + .../net/nfp/flower/nfp_flower_representor.c | 19 +++ .../net/nfp/flower/nfp_flower_representor.h | 1 + drivers/net/nfp/nfp_mtr.c | 2 +- drivers/net/nfp/nfp_net_common.c | 27 ++-- drivers/net/nfp/nfp_net_common.h | 10 ++ 11 files changed, 233 insertions(+), 17 deletions(-) -- 2.39.1
[PATCH 1/4] net/nfp: support MARK flow action
From: Long Wu
Add the corresponding logics to support the offload of MARK action.
Signed-off-by: Long Wu
Reviewed-by: Chaoyong He
Reviewed-by: Peng Zhang
---
drivers/net/nfp/flower/nfp_flower_cmsg.h | 16 ++
drivers/net/nfp/flower/nfp_flower_flow.c | 27
2 files changed, 43 insertions(+)
diff --git a/drivers/net/nfp/flower/nfp_flower_cmsg.h
b/drivers/net/nfp/flower/nfp_flower_cmsg.h
index 8fb55f44a2..c94ea706bb 100644
--- a/drivers/net/nfp/flower/nfp_flower_cmsg.h
+++ b/drivers/net/nfp/flower/nfp_flower_cmsg.h
@@ -959,6 +959,22 @@ struct nfp_fl_act_meter {
rte_be32_t profile_id;
};
+/*
+ * Mark
+ *3 2 1
+ * 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | res | opcode | res | len_lw|reserved |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Mark |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ */
+struct nfp_fl_act_mark {
+ struct nfp_fl_act_head head;
+ rte_be16_t reserved;
+ rte_be32_t mark;
+};
+
int nfp_flower_cmsg_mac_repr(struct nfp_app_fw_flower *app_fw_flower);
int nfp_flower_cmsg_repr_reify(struct nfp_app_fw_flower *app_fw_flower,
struct nfp_flower_representor *repr);
diff --git a/drivers/net/nfp/flower/nfp_flower_flow.c
b/drivers/net/nfp/flower/nfp_flower_flow.c
index e26be30d18..8667f8e901 100644
--- a/drivers/net/nfp/flower/nfp_flower_flow.c
+++ b/drivers/net/nfp/flower/nfp_flower_flow.c
@@ -83,6 +83,7 @@
#define NFP_FL_ACTION_OPCODE_METER 24
#define NFP_FL_ACTION_OPCODE_CT_NAT_EXT 25
#define NFP_FL_ACTION_OPCODE_PUSH_GENEVE26
+#define NFP_FL_ACTION_OPCODE_SET_MARK 27
#define NFP_FL_ACTION_OPCODE_NUM32
#define NFP_FL_OUT_FLAGS_LASTRTE_BIT32(15)
@@ -1138,6 +1139,10 @@ nfp_flow_key_layers_calculate_actions(const struct
rte_flow_action actions[],
case RTE_FLOW_ACTION_TYPE_CONNTRACK:
PMD_DRV_LOG(DEBUG, "RTE_FLOW_ACTION_TYPE_CONNTRACK
detected");
break;
+ case RTE_FLOW_ACTION_TYPE_MARK:
+ key_ls->act_size += sizeof(struct nfp_fl_act_mark);
+ PMD_DRV_LOG(DEBUG, "RTE_FLOW_ACTION_TYPE_MARK
detected");
+ break;
default:
PMD_DRV_LOG(ERR, "Action type %d not supported.",
action->type);
return -ENOTSUP;
@@ -3487,6 +3492,23 @@ nfp_flow_action_meter(struct nfp_flower_representor
*representor,
return 0;
}
+static void
+nfp_flow_action_mark(char *act_data,
+ const struct rte_flow_action *action)
+{
+ struct nfp_fl_act_mark *fl_mark;
+ const struct rte_flow_action_mark *mark;
+ size_t act_size = sizeof(struct nfp_fl_act_mark);
+
+ mark = action->conf;
+
+ fl_mark = (struct nfp_fl_act_mark *)act_data;
+ fl_mark->head.jump_id = NFP_FL_ACTION_OPCODE_SET_MARK;
+ fl_mark->head.len_lw = act_size >> NFP_FL_LW_SIZ;
+ fl_mark->reserved = 0;
+ fl_mark->mark = rte_cpu_to_be_32(mark->id);
+}
+
static uint32_t
nfp_flow_count_output(const struct rte_flow_action actions[])
{
@@ -3734,6 +3756,11 @@ nfp_flow_compile_action(struct nfp_flower_representor
*representor,
case RTE_FLOW_ACTION_TYPE_CONNTRACK:
PMD_DRV_LOG(DEBUG, "Process
RTE_FLOW_ACTION_TYPE_CONNTRACK");
break;
+ case RTE_FLOW_ACTION_TYPE_MARK:
+ PMD_DRV_LOG(DEBUG, "Process RTE_FLOW_ACTION_TYPE_MARK");
+ nfp_flow_action_mark(position, action);
+ position += sizeof(struct nfp_fl_act_mark);
+ break;
default:
PMD_DRV_LOG(ERR, "Unsupported action type: %d",
action->type);
return -ENOTSUP;
--
2.39.1
[PATCH 2/4] net/nfp: add interface to check representor
From: Long Wu
Add a interface to check if a device is a representor.
Signed-off-by: Long Wu
Reviewed-by: Chaoyong He
Reviewed-by: Peng Zhang
---
drivers/net/nfp/flower/nfp_flower_flow.c | 2 +-
drivers/net/nfp/nfp_mtr.c| 2 +-
drivers/net/nfp/nfp_net_common.c | 10 --
drivers/net/nfp/nfp_net_common.h | 1 +
4 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/net/nfp/flower/nfp_flower_flow.c
b/drivers/net/nfp/flower/nfp_flower_flow.c
index 8667f8e901..07833fdbdb 100644
--- a/drivers/net/nfp/flower/nfp_flower_flow.c
+++ b/drivers/net/nfp/flower/nfp_flower_flow.c
@@ -4348,7 +4348,7 @@ int
nfp_flow_ops_get(struct rte_eth_dev *dev,
const struct rte_flow_ops **ops)
{
- if ((dev->data->dev_flags & RTE_ETH_DEV_REPRESENTOR) == 0) {
+ if (!nfp_net_dev_is_repr(dev)) {
*ops = NULL;
PMD_DRV_LOG(ERR, "Port is not a representor.");
return -EINVAL;
diff --git a/drivers/net/nfp/nfp_mtr.c b/drivers/net/nfp/nfp_mtr.c
index 255977ec22..11da5c07ed 100644
--- a/drivers/net/nfp/nfp_mtr.c
+++ b/drivers/net/nfp/nfp_mtr.c
@@ -1066,7 +1066,7 @@ static const struct rte_mtr_ops nfp_mtr_ops = {
int
nfp_net_mtr_ops_get(struct rte_eth_dev *dev, void *arg)
{
- if ((dev->data->dev_flags & RTE_ETH_DEV_REPRESENTOR) == 0) {
+ if (!nfp_net_dev_is_repr(dev)) {
PMD_DRV_LOG(ERR, "Port is not a representor");
return -EINVAL;
}
diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index 72c9a41b00..7a0b0c9973 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -156,6 +156,12 @@ static const uint32_t nfp_net_link_speed_nfp2rte[] = {
[NFP_NET_CFG_STS_LINK_RATE_100G]= RTE_ETH_SPEED_NUM_100G,
};
+bool
+nfp_net_dev_is_repr(const struct rte_eth_dev *eth_dev)
+{
+ return ((eth_dev->data->dev_flags & RTE_ETH_DEV_REPRESENTOR) != 0);
+}
+
static uint16_t
nfp_net_link_speed_rte2nfp(uint16_t speed)
{
@@ -241,7 +247,7 @@ nfp_net_get_hw(const struct rte_eth_dev *dev)
{
struct nfp_net_hw *hw;
- if ((dev->data->dev_flags & RTE_ETH_DEV_REPRESENTOR) != 0) {
+ if (nfp_net_dev_is_repr(dev)) {
struct nfp_flower_representor *repr;
repr = dev->data->dev_private;
hw = repr->app_fw_flower->pf_hw;
@@ -2139,7 +2145,7 @@ nfp_net_firmware_version_get(struct rte_eth_dev *dev,
hw = nfp_net_get_hw(dev);
- if ((dev->data->dev_flags & RTE_ETH_DEV_REPRESENTOR) != 0) {
+ if (nfp_net_dev_is_repr(dev)) {
snprintf(vnic_version, FW_VER_LEN, "%d.%d.%d.%d",
hw->ver.extend, hw->ver.class,
hw->ver.major, hw->ver.minor);
diff --git a/drivers/net/nfp/nfp_net_common.h b/drivers/net/nfp/nfp_net_common.h
index e374739022..1d96b0e9d1 100644
--- a/drivers/net/nfp/nfp_net_common.h
+++ b/drivers/net/nfp/nfp_net_common.h
@@ -307,6 +307,7 @@ int nfp_net_fec_get(struct rte_eth_dev *dev,
uint32_t *fec_capa);
int nfp_net_fec_set(struct rte_eth_dev *dev,
uint32_t fec_capa);
+bool nfp_net_dev_is_repr(const struct rte_eth_dev *eth_dev);
#define NFP_PRIV_TO_APP_FW_NIC(app_fw_priv)\
((struct nfp_app_fw_nic *)app_fw_priv)
--
2.39.1
[PATCH 3/4] net/nfp: representor adds RSS configuration
From: Long Wu
Add RSS configuration in representor initializaion.
Signed-off-by: Long Wu
Reviewed-by: Chaoyong He
Reviewed-by: Peng Zhang
---
drivers/common/nfp/nfp_common_ctrl.h| 1 +
drivers/net/nfp/flower/nfp_flower.c | 12 ++--
drivers/net/nfp/flower/nfp_flower_representor.c | 13 +
drivers/net/nfp/nfp_net_common.c| 17 ++---
drivers/net/nfp/nfp_net_common.h| 9 +
5 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/drivers/common/nfp/nfp_common_ctrl.h
b/drivers/common/nfp/nfp_common_ctrl.h
index d65fcd17cb..d0a3fc696c 100644
--- a/drivers/common/nfp/nfp_common_ctrl.h
+++ b/drivers/common/nfp/nfp_common_ctrl.h
@@ -253,6 +253,7 @@ struct nfp_net_fw_ver {
#define NFP_NET_CFG_RSS_IPV4_SCTP (1 << 14) /* RSS for IPv4/SCTP */
#define NFP_NET_CFG_RSS_IPV6_SCTP (1 << 15) /* RSS for IPv6/SCTP */
#define NFP_NET_CFG_RSS_TOEPLITZ(1 << 24) /* Use Toeplitz hash */
+#define NFP_NET_CFG_RSS_CRC32 (1 << 26) /* Use CRC32 hash */
#define NFP_NET_CFG_RSS_KEY (NFP_NET_CFG_RSS_BASE + 0x4)
#define NFP_NET_CFG_RSS_KEY_SZ 0x28
#define NFP_NET_CFG_RSS_ITBL(NFP_NET_CFG_RSS_BASE + 0x4 + \
diff --git a/drivers/net/nfp/flower/nfp_flower.c
b/drivers/net/nfp/flower/nfp_flower.c
index e84e6ebbff..c6a744e868 100644
--- a/drivers/net/nfp/flower/nfp_flower.c
+++ b/drivers/net/nfp/flower/nfp_flower.c
@@ -31,6 +31,8 @@ nfp_flower_pf_start(struct rte_eth_dev *dev)
uint32_t new_ctrl;
uint32_t update = 0;
struct nfp_net_hw *net_hw;
+ struct rte_eth_conf *dev_conf;
+ struct rte_eth_rxmode *rxmode;
struct nfp_flower_representor *repr;
repr = dev->data->dev_private;
@@ -48,8 +50,14 @@ nfp_flower_pf_start(struct rte_eth_dev *dev)
/* Writing configuration parameters in the device */
nfp_net_params_setup(net_hw);
- update |= NFP_NET_CFG_UPDATE_RSS;
- new_ctrl |= nfp_net_cfg_ctrl_rss(hw->cap);
+ dev_conf = &dev->data->dev_conf;
+ rxmode = &dev_conf->rxmode;
+
+ if ((rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) != 0) {
+ nfp_net_rss_config_default(dev);
+ update |= NFP_NET_CFG_UPDATE_RSS;
+ new_ctrl |= nfp_net_cfg_ctrl_rss(hw->cap);
+ }
/* Enable device */
new_ctrl |= NFP_NET_CFG_CTRL_ENABLE;
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c
b/drivers/net/nfp/flower/nfp_flower_representor.c
index 4f4df0cd2e..7284a1e84d 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -44,6 +44,12 @@ static int
nfp_flower_repr_dev_infos_get(__rte_unused struct rte_eth_dev *dev,
struct rte_eth_dev_info *dev_info)
{
+ struct nfp_net_hw *pf_hw;
+ struct nfp_flower_representor *repr;
+
+ repr = dev->data->dev_private;
+ pf_hw = repr->app_fw_flower->pf_hw;
+
/* Hardcoded pktlen and queues for now */
dev_info->max_rx_queues = 1;
dev_info->max_tx_queues = 1;
@@ -64,6 +70,13 @@ nfp_flower_repr_dev_infos_get(__rte_unused struct
rte_eth_dev *dev,
dev_info->max_mac_addrs = 1;
+ if ((pf_hw->super.cap & NFP_NET_CFG_CTRL_RSS_ANY) != 0) {
+ dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_RSS_HASH;
+ dev_info->flow_type_rss_offloads = NFP_NET_RSS_CAP;
+ dev_info->reta_size = NFP_NET_CFG_RSS_ITBL_SZ;
+ dev_info->hash_key_size = NFP_NET_CFG_RSS_KEY_SZ;
+ }
+
return 0;
}
diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index 7a0b0c9973..66aaf7ab09 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -1269,16 +1269,7 @@ nfp_net_infos_get(struct rte_eth_dev *dev, struct
rte_eth_dev_info *dev_info)
if ((cap & NFP_NET_CFG_CTRL_RSS_ANY) != 0) {
dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_RSS_HASH;
-
- dev_info->flow_type_rss_offloads = RTE_ETH_RSS_IPV4 |
- RTE_ETH_RSS_NONFRAG_IPV4_TCP |
- RTE_ETH_RSS_NONFRAG_IPV4_UDP |
- RTE_ETH_RSS_NONFRAG_IPV4_SCTP |
- RTE_ETH_RSS_IPV6 |
- RTE_ETH_RSS_NONFRAG_IPV6_TCP |
- RTE_ETH_RSS_NONFRAG_IPV6_UDP |
- RTE_ETH_RSS_NONFRAG_IPV6_SCTP;
-
+ dev_info->flow_type_rss_offloads = NFP_NET_RSS_CAP;
dev_info->reta_size = NFP_NET_CFG_RSS_ITBL_SZ;
dev_info->hash_key_size = NFP_NET_CFG_RSS_KEY_SZ;
}
@@ -1794,7 +1785,11 @@ nfp_net_rss_hash_write(struct rte_eth_dev *dev,
cfg_rss_ctrl |= NFP_NET_CFG_RSS_IPV6_SCTP;
cfg_rss_ctrl |= NFP_NET_CFG_RSS_MASK;
- cfg_rss_ctrl |= NFP_NET_CFG_R
[PATCH 4/4] net/nfp: support RSS flow action
From: Long Wu
Add the corresponding logics to support the offload of RSS action.
Signed-off-by: Long Wu
Reviewed-by: Chaoyong He
Reviewed-by: Peng Zhang
---
doc/guides/nics/features/nfp.ini | 1 +
drivers/net/nfp/flower/nfp_flower_flow.c | 124 ++
drivers/net/nfp/flower/nfp_flower_flow.h | 8 ++
.../net/nfp/flower/nfp_flower_representor.c | 6 +
.../net/nfp/flower/nfp_flower_representor.h | 1 +
5 files changed, 140 insertions(+)
diff --git a/doc/guides/nics/features/nfp.ini b/doc/guides/nics/features/nfp.ini
index 3494111f86..b20049b4f5 100644
--- a/doc/guides/nics/features/nfp.ini
+++ b/doc/guides/nics/features/nfp.ini
@@ -59,6 +59,7 @@ queue= Y
raw_decap= Y
raw_encap= Y
represented_port = Y
+rss = Y
port_id = Y
set_ipv4_dscp= Y
set_ipv4_dst = Y
diff --git a/drivers/net/nfp/flower/nfp_flower_flow.c
b/drivers/net/nfp/flower/nfp_flower_flow.c
index 07833fdbdb..1e208b9107 100644
--- a/drivers/net/nfp/flower/nfp_flower_flow.c
+++ b/drivers/net/nfp/flower/nfp_flower_flow.c
@@ -1143,6 +1143,9 @@ nfp_flow_key_layers_calculate_actions(const struct
rte_flow_action actions[],
key_ls->act_size += sizeof(struct nfp_fl_act_mark);
PMD_DRV_LOG(DEBUG, "RTE_FLOW_ACTION_TYPE_MARK
detected");
break;
+ case RTE_FLOW_ACTION_TYPE_RSS:
+ PMD_DRV_LOG(DEBUG, "RTE_FLOW_ACTION_TYPE_RSS detected");
+ break;
default:
PMD_DRV_LOG(ERR, "Action type %d not supported.",
action->type);
return -ENOTSUP;
@@ -3509,6 +3512,116 @@ nfp_flow_action_mark(char *act_data,
fl_mark->mark = rte_cpu_to_be_32(mark->id);
}
+static int
+nfp_flow_action_rss_add(struct nfp_flower_representor *representor,
+ const struct rte_flow_action *action,
+ struct nfp_fl_rss **rss_store)
+{
+ int ret;
+ struct nfp_net_hw *pf_hw;
+ struct rte_eth_rss_conf rss_conf;
+ struct nfp_fl_rss *rss_store_tmp;
+ const struct rte_flow_action_rss *rss;
+ uint8_t rss_key[NFP_NET_CFG_RSS_KEY_SZ];
+
+ if (nfp_flower_repr_is_vf(representor))
+ return 0;
+
+ rss = action->conf;
+
+ if (rss->key_len > NFP_NET_CFG_RSS_KEY_SZ) {
+ PMD_DRV_LOG(ERR, "Unsupported rss key length.");
+ return -ENOTSUP;
+ }
+
+ rss_conf.rss_hf = 0;
+ rss_conf.rss_key = rss_key;
+ pf_hw = representor->app_fw_flower->pf_hw;
+ ret = nfp_net_rss_hash_conf_get(pf_hw->eth_dev, &rss_conf);
+ if (ret != 0) {
+ PMD_DRV_LOG(ERR, "Get RSS conf failed.");
+ return ret;
+ }
+
+ rss_store_tmp = calloc(1, sizeof(struct nfp_fl_rss));
+ if (rss_store_tmp == NULL) {
+ PMD_DRV_LOG(ERR, "Alloc memory for rss storage failed.");
+ return -ENOMEM;
+ }
+
+ if (rss->types != 0) {
+ rss_conf.rss_hf |= rss->types;
+
+ rss_store_tmp->types = rss->types;
+ }
+
+ if (rss->key_len != 0 && rss->key != NULL) {
+ memcpy(rss_conf.rss_key, rss->key, rss->key_len);
+ rss_conf.rss_key_len = rss->key_len;
+
+ memcpy(rss_store_tmp->key, rss->key, rss->key_len);
+ rss_store_tmp->key_len = rss->key_len;
+ }
+
+ ret = nfp_net_rss_hash_update(pf_hw->eth_dev, &rss_conf);
+ if (ret != 0) {
+ PMD_DRV_LOG(ERR, "Update RSS conf failed.");
+ free(rss_store_tmp);
+ return ret;
+ }
+
+ *rss_store = rss_store_tmp;
+
+ return 0;
+}
+
+static int
+nfp_flow_action_rss_del(struct nfp_flower_representor *representor,
+ struct rte_flow *nfp_flow)
+{
+ int ret;
+ struct nfp_net_hw *pf_hw;
+ struct nfp_fl_rss *rss_store;
+ struct rte_eth_rss_conf rss_conf;
+ uint8_t rss_key[NFP_NET_CFG_RSS_KEY_SZ];
+
+ if (nfp_flower_repr_is_vf(representor))
+ return 0;
+
+ rss_conf.rss_hf = 0;
+ rss_conf.rss_key = rss_key;
+ pf_hw = representor->app_fw_flower->pf_hw;
+ ret = nfp_net_rss_hash_conf_get(pf_hw->eth_dev, &rss_conf);
+ if (ret != 0) {
+ PMD_DRV_LOG(ERR, "Get RSS conf failed.");
+ goto exit;
+ }
+
+ rss_store = nfp_flow->rss;
+
+ if ((rss_conf.rss_hf & rss_store->types) != 0)
+ rss_conf.rss_hf &= (~(rss_store->types));
+
+ /* Need default RSS configuration */
+ if (rss_conf.rss_hf == 0)
+ rss_conf.rss_hf = RTE_ETH_RSS_IPV4 | RTE_ETH_RSS_IPV6;
+
+ if (rss_conf.rss_key_len == rss_store->key_len &&
+ memcmp(rss_conf.rss_key, rss_store->key,
rss_store->key_len) == 0) {
+ rss_conf.rss_
Re: [PATCH v2] dmadev: standardize alignment
Acked-by: Chengwen Feng
On 2024/2/10 14:27, pbhagavat...@marvell.com wrote:
From: Pavan Nikhilesh
Align fp_objects based on cacheline size defined
by build configuration.
Signed-off-by: Pavan Nikhilesh
---
v2 Changes:
- Drop malloc changes.
lib/dmadev/rte_dmadev_core.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/dmadev/rte_dmadev_core.h b/lib/dmadev/rte_dmadev_core.h
index 064785686f..e8239c2d22 100644
--- a/lib/dmadev/rte_dmadev_core.h
+++ b/lib/dmadev/rte_dmadev_core.h
@@ -73,7 +73,7 @@ struct rte_dma_fp_object {
rte_dma_completed_tcompleted;
rte_dma_completed_status_t completed_status;
rte_dma_burst_capacity_t burst_capacity;
-} __rte_aligned(128);
+} __rte_cache_aligned;
extern struct rte_dma_fp_object *rte_dma_fp_objs;
--
2.25.1
Re: [PATCH v3 1/3] config/arm: avoid mcpu and march conflicts
> On Feb 10, 2024, at 12:49 AM, Pavan Nikhilesh Bhagavatula > wrote: > > wrote: > > Hi Pavan, > >> The compiler options march and mtune are a subset of mcpu and will >> lead to >> conflicts if improper march is chosen for a given mcpu. >> To avoid conflicts, force part number march when mcpu is available >> and is supported by the compiler. > > Why would one force the march specified in the part number when mcpu > for that part number is also available and supported by the compiler? > It would be good to explain the use case or the problem being faced. >>> >>> The idea of this patchset is to avoid mcpu and march conflicts that can >> happen >>> with the current build flow. >>> >>> #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.6-a shrn.c >>> cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.6-a' >>> >>> In some versions of GCC mcpu=neoverse-n2 is supported but - >> march=armv9- >>> a is not so, current build flow will choose the next supported march which >>> is >>> armv8.6-a and report a conflict. >>> >> If compiler support is available for a certain CPU, then it is safe to >> assume that >> the >> Compiler knows the best architecture to use. >> Therefore, in such cases the best practice is to not provide -march. >> > > Ok, this deviates a lot from the current build flow, I will rework and send a > v4. > The overall goal is to ensure the user knows that they are making suboptimal choices. We will do some canned build configurations as follows: 1) Architecture build configurations - generic, generic-v9 2) Core configurations - n2, v2 etc 3) SoC configurations - Octeon 10, Bluefield 3 etc These will provide flexibility on portability and performance. Outside of these, there will be a lot of permutations/combinations of architecture flags supported by compiler and CPU. It does not make sense to support all of them. However, we have to provide a method to the users to use whatever flags they want. We will do that by using the command line parameters as overrides. Please let us know if you see problems in this approach. We also need to ensure we are not breaking any backward compatibility here. We (Arm) will document this in the linux guide. >> >> Example: >> march = armv9-a >> mcpu = neoverse-n2 >> >> mcpu supported, march supported >> machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a'] > > -march restricts the compiler to baseline architecture of the -mcpu. > For instance, Neoverse-n1's baseline architecture is armv8.2-a, but > it has some extensions from armv8.3-a, armv8.4-a, and armv8.5-a. > By setting -march to armv8.2-a the compiler will strictly omit > extensions from 8.3, 8.4 and 8.5 resulting in a suboptimal outcome. >>> >>> What if compiler only supports armv8.2-a? >>> Are you suggesting we don’t use march at all when mcpu is supported? >>> If so how do you express extensions that the SoC supports? >>> Neoverse-n2 has optional support for crypto and can only be enabled by >>> expressing it through march='armv9-a+crypto' >>> >> March extensions also works with mcpu, use mcpu=neoverse-n2+crypto >> instead of march. It's documented in "-march and -mcpu Feature Modifiers" >> section in gcc manual. >> > >> >> mcpu supported, march not supported machine_args = >> ['-mcpu=neoverse-n2'] > > This will result in the best outcome. >>> >>> Isn't -mcpu=neoverse-n2 -march=armv9-a+sve2+crypto also the best >>> outcome? >>> >> Here also we can append feature modifiers like sve2 and crypto to CPU in >> -mcpu and drop -march arg. >> If the compiler supports neoverse-n2 but not armv9-a it will pick the next >> best architecture. >> -mcpu=neoverse-n2+sve2+crypto can replace - march=armv9-a+sve2+crypto >> > >> >> mcpu not supported, march supported machine_args = >> ['-march=armv9-a'] > > This too may result in a suboptimal outcome as optimization space is > limited to the given march (not using extensions from later > architectures when available). > >>> >>> What if compiler doesn’t support mcpu=neoverse-n2 and only supports >>> march=armv9-a >>> >> I agree there can be such corner cases where CPU enablement isn't done. >> Such cases can be handled with a new meson build parameter like >> -Dplatform=generic-armv9 to build armv9-a binaries (similar to >> -Dplatform=generic that builds armv8-a binaries today). >> Having such parameter forces the user to make a conscious decision rather >> than build system doing it for them. >> It also comes with the added benefit of having a simpler build system. >> >> >> mcpu not supported, march not supported machine_args = >> ['-march=armv8.6-a'] > > Compiler knows nothing about the target CPU or the architecture. > I think it's better to exit the build process with an error. > >>> >>> Then we
Re: [PATCH v2 3/3] config/arm: allow WFE to be enabled config time
> On Feb 10, 2024, at 12:47 AM, Pavan Nikhilesh Bhagavatula > wrote: > >>> On Feb 1, 2024, at 3:57 PM, pbhagavat...@marvell.com wrote: >>> >>> From: Pavan Nikhilesh >>> >>> Allow RTE_ARM_USE_WFE to be enabled at meson configuration >>> time by passing it via c_args instead of modifying >>> `config/arm/meson.build`. >>> >>> Example usage: >>> meson build -Dc_args='-DRTE_ARM_USE_WFE' \ >>> --cross-file config/arm/arm64_cn10k_linux_gcc >>> >>> Signed-off-by: Pavan Nikhilesh >>> Acked-by: Chengwen Feng >>> Acked-by: Ruifeng Wang >>> --- >>> config/arm/meson.build | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/config/arm/meson.build b/config/arm/meson.build >>> index 6f2308f2fa..3467bef466 100644 >>> --- a/config/arm/meson.build >>> +++ b/config/arm/meson.build >>> @@ -17,7 +17,9 @@ flags_common = [ >>>#['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF], >>>#['RTE_ARM64_MEMCPY_STRICT_ALIGN', false], >>> >>> -['RTE_ARM_USE_WFE', false], >>> +# Enable use of ARM wait for event instruction. >>> +# ['RTE_ARM_USE_WFE', false], >>> + >> So, what is the default value for RTE_ARM_USE_WFE if the user does not pass >> the flag at the command line? >> > > All the checks related to RTE_ARM_USE_WFE see if it is defined or not > > #rg "RTE_ARM_USE_WFE" > config/arm/meson.build > 20:['RTE_ARM_USE_WFE', false], > lib/eal/arm/rte_cpuflags.c > 166:#ifdef RTE_ARM_USE_WFE > lib/eal/arm/include/rte_pause_64.h > 15:#ifdef RTE_ARM_USE_WFE > lib/eal/arm/rte_power_intrinsics.c > 20:#ifdef RTE_ARM_USE_WFE > 84:#ifdef RTE_ARM_USE_WFE > > So default value would be not-defined. > >> Can we do it such a way that the flag passed on the command line takes >> precedence? > > No, we can't have checks based on flags passed via -Dc_args with the current > meson(needs build_options() from 1.1.0). > Only option is to add via meson_option.txt which is not optimal for arch > settings. Ack Do you expect the users to enable this flag by default for cn10k? > >> >>>['RTE_ARCH_ARM64', true], >>>['RTE_CACHE_LINE_SIZE', 128] >>> ] >>> -- >>> 2.25.1 >>> >
RE: [PATCH v2 3/3] config/arm: allow WFE to be enabled config time
> -Original Message- > From: Honnappa Nagarahalli > Sent: Saturday, February 10, 2024 10:07 PM > To: Pavan Nikhilesh Bhagavatula > Cc: Jerin Jacob ; juraj.lin...@pantheon.tech; nd > ; Ruifeng Wang ; Bruce Richardson > ; dev@dpdk.org; Chengwen Feng > ; Wathsala Wathawana Vithanage > ; Dhruv Tripathi > Subject: [EXT] Re: [PATCH v2 3/3] config/arm: allow WFE to be enabled config > time > > External Email > > -- > > > > On Feb 10, 2024, at 12:47 AM, Pavan Nikhilesh Bhagavatula > wrote: > > > >>> On Feb 1, 2024, at 3:57 PM, pbhagavat...@marvell.com wrote: > >>> > >>> From: Pavan Nikhilesh > >>> > >>> Allow RTE_ARM_USE_WFE to be enabled at meson configuration > >>> time by passing it via c_args instead of modifying > >>> `config/arm/meson.build`. > >>> > >>> Example usage: > >>> meson build -Dc_args='-DRTE_ARM_USE_WFE' \ > >>> --cross-file config/arm/arm64_cn10k_linux_gcc > >>> > >>> Signed-off-by: Pavan Nikhilesh > >>> Acked-by: Chengwen Feng > >>> Acked-by: Ruifeng Wang > >>> --- > >>> config/arm/meson.build | 4 +++- > >>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/config/arm/meson.build b/config/arm/meson.build > >>> index 6f2308f2fa..3467bef466 100644 > >>> --- a/config/arm/meson.build > >>> +++ b/config/arm/meson.build > >>> @@ -17,7 +17,9 @@ flags_common = [ > >>>#['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF], > >>>#['RTE_ARM64_MEMCPY_STRICT_ALIGN', false], > >>> > >>> -['RTE_ARM_USE_WFE', false], > >>> +# Enable use of ARM wait for event instruction. > >>> +# ['RTE_ARM_USE_WFE', false], > >>> + > >> So, what is the default value for RTE_ARM_USE_WFE if the user does not > pass > >> the flag at the command line? > >> > > > > All the checks related to RTE_ARM_USE_WFE see if it is defined or not > > > > #rg "RTE_ARM_USE_WFE" > > config/arm/meson.build > > 20:['RTE_ARM_USE_WFE', false], > > lib/eal/arm/rte_cpuflags.c > > 166:#ifdef RTE_ARM_USE_WFE > > lib/eal/arm/include/rte_pause_64.h > > 15:#ifdef RTE_ARM_USE_WFE > > lib/eal/arm/rte_power_intrinsics.c > > 20:#ifdef RTE_ARM_USE_WFE > > 84:#ifdef RTE_ARM_USE_WFE > > > > So default value would be not-defined. > > > >> Can we do it such a way that the flag passed on the command line takes > >> precedence? > > > > No, we can't have checks based on flags passed via -Dc_args with the current > meson(needs build_options() from 1.1.0). > > Only option is to add via meson_option.txt which is not optimal for arch > settings. > > Ack > Do you expect the users to enable this flag by default for cn10k? > No, AFAIK only certain users are going to enable it, mostly L1. > > > >> > >>>['RTE_ARCH_ARM64', true], > >>>['RTE_CACHE_LINE_SIZE', 128] > >>> ] > >>> -- > >>> 2.25.1 > >>> > >
Re: [PATCH v3 3/3] config/arm: allow WFE to be enabled config time
> On Feb 2, 2024, at 2:50 AM, pbhagavat...@marvell.com wrote: > > From: Pavan Nikhilesh > > Allow RTE_ARM_USE_WFE to be enabled at meson configuration > time by passing it via c_args instead of modifying > `config/arm/meson.build`. > > Example usage: > meson build -Dc_args='-DRTE_ARM_USE_WFE' \ > --cross-file config/arm/arm64_cn10k_linux_gcc > > Signed-off-by: Pavan Nikhilesh > Acked-by: Chengwen Feng > Acked-by: Ruifeng Wang Reviewed-by: Honnappa Nagarahalli --- > config/arm/meson.build | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/config/arm/meson.build b/config/arm/meson.build > index 4e44d1850bae..01870a23328a 100644 > --- a/config/arm/meson.build > +++ b/config/arm/meson.build > @@ -17,7 +17,9 @@ flags_common = [ > #['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF], > #['RTE_ARM64_MEMCPY_STRICT_ALIGN', false], > > -['RTE_ARM_USE_WFE', false], > +# Enable use of ARM wait for event instruction. > +# ['RTE_ARM_USE_WFE', false], > + > ['RTE_ARCH_ARM64', true], > ['RTE_CACHE_LINE_SIZE', 128] > ] > -- > 2.43.0 >
Re: [PATCH v3 1/3] config/arm: avoid mcpu and march conflicts
> On Feb 10, 2024, at 9:20 AM, Honnappa Nagarahalli > wrote: > > > >> On Feb 10, 2024, at 12:49 AM, Pavan Nikhilesh Bhagavatula >> wrote: >> >> > wrote: >> >> Hi Pavan, >> >>> The compiler options march and mtune are a subset of mcpu and will >>> lead > to >>> conflicts if improper march is chosen for a given mcpu. >>> To avoid conflicts, force part number march when mcpu is available >>> and is supported by the compiler. >> >> Why would one force the march specified in the part number when mcpu >> for that part number is also available and supported by the compiler? >> > It would be good to explain the use case or the problem being faced. > The idea of this patchset is to avoid mcpu and march conflicts that can >>> happen with the current build flow. #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.6-a shrn.c cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.6-a' In some versions of GCC mcpu=neoverse-n2 is supported but - >>> march=armv9- a is not so, current build flow will choose the next supported march which is armv8.6-a and report a conflict. >>> If compiler support is available for a certain CPU, then it is safe to >>> assume that >>> the >>> Compiler knows the best architecture to use. >>> Therefore, in such cases the best practice is to not provide -march. >>> >> >> Ok, this deviates a lot from the current build flow, I will rework and send >> a v4. >> > The overall goal is to ensure the user knows that they are making suboptimal > choices. We will do some canned build configurations as follows: > > 1) Architecture build configurations - generic, generic-v9 > 2) Core configurations - n2, v2 etc > 3) SoC configurations - Octeon 10, Bluefield 3 etc > > These will provide flexibility on portability and performance. > > Outside of these, there will be a lot of permutations/combinations of > architecture flags supported by compiler and CPU. It does not make sense to > support all of them. > > However, we have to provide a method to the users to use whatever flags they > want. We will do that by using the command line parameters as overrides. > > Please let us know if you see problems in this approach. We also need to > ensure we are not breaking any backward compatibility here. > > We (Arm) will document this in the linux guide. Currently, we are displaying warnings when there is a mismatch between the platform the user chooses and what the compiler supports. For ex: if the user wants to compile for N2 (-Dplatform=n2) and the compiler does not support -mcpu=neoverse-n2, we display a warning and the build continues. I think we need to change this to error forcing the user to update the compiler or building for a generic v8 or generic-v9 or asking them to provide options at the command line. > >>> >>> Example: >>> march = armv9-a >>> mcpu = neoverse-n2 >>> >>> mcpu supported, march supported >>> machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a'] >> >> -march restricts the compiler to baseline architecture of the -mcpu. >> For instance, Neoverse-n1's baseline architecture is armv8.2-a, but >> it has some extensions from armv8.3-a, armv8.4-a, and armv8.5-a. >> By setting -march to armv8.2-a the compiler will strictly omit >> extensions from 8.3, 8.4 and 8.5 resulting in a suboptimal outcome. What if compiler only supports armv8.2-a? Are you suggesting we don’t use march at all when mcpu is supported? If so how do you express extensions that the SoC supports? Neoverse-n2 has optional support for crypto and can only be enabled by expressing it through march='armv9-a+crypto' >>> March extensions also works with mcpu, use mcpu=neoverse-n2+crypto >>> instead of march. It's documented in "-march and -mcpu Feature Modifiers" >>> section in gcc manual. >>> >> >>> >>> mcpu supported, march not supported machine_args = >>> ['-mcpu=neoverse-n2'] >> >> This will result in the best outcome. Isn't -mcpu=neoverse-n2 -march=armv9-a+sve2+crypto also the best outcome? >>> Here also we can append feature modifiers like sve2 and crypto to CPU in >>> -mcpu and drop -march arg. >>> If the compiler supports neoverse-n2 but not armv9-a it will pick the next >>> best architecture. >>> -mcpu=neoverse-n2+sve2+crypto can replace - march=armv9-a+sve2+crypto >>> >> >>> >>> mcpu not supported, march supported machine_args = >>> ['-march=armv9-a'] >> >> This too may result in a suboptimal outcome as optimization space is >> limited to the given march (not using extensions from later >> architectures when available). >> What if compiler doesn’t support mcpu=neoverse-n2 and only supports march=armv9-a >>> I agree there can be such corner ca
[PATCH v6] gro : packets not getting flushed in heavy-weight mode API
In heavy-weight mode GRO which is based on timer, the GRO packets
will not be flushed in spite of timer expiry if there is no packet
in the current poll. If timer mode GRO is enabled the
rte_gro_timeout_flush API should be invoked.
Signed-off-by: Kumara Parameshwaran
---
v1:
Changes to make sure that the GRO flush API is invoked if there are no
packets in
current poll and timer expiry.
v2:
Fix code organisation issue
v3:
Fix warnings
v4:
Fix error and warnings
v5:
Fix compilation issue when GRO is not defined
v6:
Address review comments
app/test-pmd/csumonly.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index c103e54111..21b45b4ba4 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -863,16 +863,22 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
/* receive a burst of packet */
nb_rx = common_fwd_stream_receive(fs, pkts_burst, nb_pkt_per_burst);
+#ifndef RTE_LIB_GRO
if (unlikely(nb_rx == 0))
return false;
-
+#else
+ gro_enable = gro_ports[fs->rx_port].enable;
+ if (unlikely(nb_rx == 0)) {
+ if (!gro_enable || (gro_flush_cycles ==
GRO_DEFAULT_FLUSH_CYCLES))
+ return false;
+ if ((rte_gro_get_pkt_count(current_fwd_lcore()->gro_ctx) == 0))
+ return false;
+ }
+#endif
rx_bad_ip_csum = 0;
rx_bad_l4_csum = 0;
rx_bad_outer_l4_csum = 0;
rx_bad_outer_ip_csum = 0;
-#ifdef RTE_LIB_GRO
- gro_enable = gro_ports[fs->rx_port].enable;
-#endif
txp = &ports[fs->tx_port];
tx_offloads = txp->dev_conf.txmode.offloads;
--
2.25.1
RE: [PATCH v2 1/4] ethdev: introduce encap hash calculation
Hi Ferruh,
> -Original Message-
> From: Ferruh Yigit
> Sent: Thursday, February 8, 2024 7:13 PM
> To: Ori Kam ; Dariusz Sosnowski
>
> On 2/8/2024 9:09 AM, Ori Kam wrote:
> > During encapsulation of a packet, it is possible to change some
> > outer headers to improve flow destribution.
> > For example, from VXLAN RFC:
> > "It is recommended that the UDP source port number
> > be calculated using a hash of fields from the inner packet --
> > one example being a hash of the inner Ethernet frame's headers.
> > This is to enable a level of entropy for the ECMP/load-balancing"
> >
> > The tunnel protocol defines which outer field should hold this hash,
> > but it doesn't define the hash calculation algorithm.
> >
> > An application that uses flow offloads gets the first few packets
> > (exception path) and then decides to offload the flow.
> > As a result, there are two
> > different paths that a packet from a given flow may take.
> > SW for the first few packets or HW for the rest.
> > When the packet goes through the SW, the SW encapsulates the packet
> > and must use the same hash calculation as the HW will do for
> > the rest of the packets in this flow.
> >
> > the new function rte_flow_calc_encap_hash can query the hash value
> > fromm the driver for a given packet as if the packet was passed
> > through the HW.
> >
> > Signed-off-by: Ori Kam
> > Acked-by: Dariusz Sosnowski
> >
>
> <...>
>
> > +int
> > +rte_flow_calc_encap_hash(uint16_t port_id, const struct rte_flow_item
> pattern[],
> > +enum rte_flow_encap_hash_field dest_field, uint8_t
> hash_len,
> > +uint8_t *hash, struct rte_flow_error *error)
> > +{
> > + int ret;
> > + struct rte_eth_dev *dev;
> > + const struct rte_flow_ops *ops;
> > +
> > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > + ops = rte_flow_ops_get(port_id, error);
> > + if (!ops || !ops->flow_calc_encap_hash)
> > + return rte_flow_error_set(error, ENOTSUP,
> > +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> > + "calc encap hash is not supported");
> > + if ((dest_field == RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT &&
> hash_len != 2) ||
> > + (dest_field == RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID
> && hash_len != 1))
> >
>
> If there is a fixed mapping with the dest_field and the size, instead of
> putting this information into check code, what do you think to put it
> into the data structure?
>
> I mean instead of using enum for dest_filed, it can be a struct that is
> holding enum and its expected size, this clarifies what the expected
> size for that field.
>
From my original email I think we only need the type, we don't need the size.
On the RFC thread there was an objection. So I added the size,
If you think it is not needed lets remove it.
> > + return rte_flow_error_set(error, EINVAL,
> > +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> > + "hash len doesn't match the
> requested field len");
> > + dev = &rte_eth_devices[port_id];
> > + ret = ops->flow_calc_encap_hash(dev, pattern, dest_field, hash,
> error);
> >
>
> 'hash_len' is get by API, but it is not passed to dev_ops, does this
> mean this information hardcoded in the driver as well, if so why
> duplicate this information in driver instead off passing hash_len to driver?
Not sure I understand, like I wrote above this is pure verification from my
point of view.
The driver knows the size based on the dest.
>
>
> > + return flow_err(port_id, ret, error);
> > +}
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > index 1267c146e5..2bdf3a4a17 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -6783,6 +6783,57 @@ rte_flow_calc_table_hash(uint16_t port_id,
> const struct rte_flow_template_table
> > const struct rte_flow_item pattern[], uint8_t
> pattern_template_index,
> > uint32_t *hash, struct rte_flow_error *error);
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Destination field type for the hash calculation, when encap action is
> used.
> > + *
> > + * @see function rte_flow_calc_encap_hash
> > + */
> > +enum rte_flow_encap_hash_field {
> > + /* Calculate hash placed in UDP source port field. */
> > + RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT,
> > + /* Calculate hash placed in NVGRE flow ID field. */
> > + RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID,
> > +};
> >
>
> Indeed above enum represents a field in a network protocol, right?
> Instead of having a 'RTE_FLOW_ENCAP_HASH_' specific one, can re-using
> 'enum rte_flow_field_id' work?
Since the option are really limited and defined by standard, I prefer to have
dedicated options.
Best,
Ori
