Patch Comment List
List patch comments
GET /api/patches/73573/comments/?format=api
[ { "id": 115665, "web_url": "https://patches.dpdk.org/comment/115665/", "msgid": "<039ED4275CED7440929022BC67E706115485A08F@SHSMSX103.ccr.corp.intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/039ED4275CED7440929022BC67E706115485A08F@SHSMSX103.ccr.corp.intel.com", "date": "2020-07-10T02:50:30", "subject": "Re: [dpdk-dev] [PATCH V2] net/i40e: i40e FDIR update rate\n\toptimization", "submitter": { "id": 504, "url": "https://patches.dpdk.org/api/people/504/?format=api", "name": "Qi Zhang", "email": "qi.z.zhang@intel.com" }, "content": "> -----Original Message-----\n> From: Sun, Chenmin <chenmin.sun@intel.com>\n> Sent: Thursday, July 9, 2020 10:40 PM\n> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>;\n> Wu, Jingjing <jingjing.wu@intel.com>; Wang, Haiyue\n> <haiyue.wang@intel.com>\n> Cc: dev@dpdk.org; Sun, Chenmin <chenmin.sun@intel.com>\n> Subject: [PATCH V2] net/i40e: i40e FDIR update rate optimization\n> \n> From: Chenmin Sun <chenmin.sun@intel.com>\n> \n> This patch optimized the fdir update rate for i40e PF, by tracking whether the\n> fdir rule being inserted into the guaranteed space or shared space.\n> For the flows that are inserted to the guaranteed space, we assume that the\n> insertion will always succeed as the hardware only reports the \"no enough\n> space left\" error. In this case, the software can directly return success and no\n> need to retrieve the result from the hardware. See the fdir programming\n> status descriptor format in the datasheet for more details.\n> \n> This patch changes the global register GLQF_CTL. Therefore, when devarg\n> ``support-multi-driver`` is set, the patch will not take effect to avoid affecting\n> the normal behavior of other i40e drivers, e.g., Linux kernel driver.\n\nOverall I think the patch is too big, is that possible to separate into 2 or more patches?\n\nFor example:\n1.) you introduce some new data structure to tack the flow\n2) the optimization for flow programming.\n\nMore comments inline.\n> \n> Signed-off-by: Chenmin Sun <chenmin.sun@intel.com>\n> ---\n> \n> v2:\n> * Refine commit message and code comments.\n> * Refine code style.\n> * Fixed several memory free bugs.\n> * Replace the bin_serch() with rte_bsf64()\n> ---\n> drivers/net/i40e/i40e_ethdev.c | 136 ++++++++++++++++++++++-\n> drivers/net/i40e/i40e_ethdev.h | 63 ++++++++---\n> drivers/net/i40e/i40e_fdir.c | 190 +++++++++++++++++++++-----------\n> drivers/net/i40e/i40e_flow.c | 167 ++++++++++++++++++++++------\n> drivers/net/i40e/i40e_rxtx.c | 15 ++-\n> drivers/net/i40e/rte_pmd_i40e.c | 2 +-\n> 6 files changed, 455 insertions(+), 118 deletions(-)\n> \n> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c\n> index 3bc312c11..099f4c5e3 100644\n> --- a/drivers/net/i40e/i40e_ethdev.c\n> +++ b/drivers/net/i40e/i40e_ethdev.c\n> @@ -26,6 +26,7 @@\n> #include <rte_dev.h>\n> #include <rte_tailq.h>\n> #include <rte_hash_crc.h>\n> +#include <rte_bitmap.h>\n> \n> #include \"i40e_logs.h\"\n> #include \"base/i40e_prototype.h\"\n> @@ -1045,8 +1046,17 @@ static int\n> i40e_init_fdir_filter_list(struct rte_eth_dev *dev) {\n> \tstruct i40e_pf *pf =\n> I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);\n> +\tstruct i40e_hw *hw = I40E_PF_TO_HW(pf);\n> \tstruct i40e_fdir_info *fdir_info = &pf->fdir;\n> \tchar fdir_hash_name[RTE_HASH_NAMESIZE];\n> +\tvoid *mem = NULL;\n> +\tuint32_t i = 0;\n> +\tuint32_t bmp_size;\n> +\tuint32_t alloc = 0;\n> +\tuint32_t best = 0;\n> +\tuint32_t pfqf_fdalloc = 0;\n> +\tuint32_t glqf_ctl_reg = 0;\n> +\tstruct rte_bitmap *bmp = NULL;\n> \tint ret;\n\nIts better to follow RCT, and please apply on other functions.\n\n> \n> \tstruct rte_hash_parameters fdir_hash_params = { @@ -1067,6 +1077,7\n> @@ i40e_init_fdir_filter_list(struct rte_eth_dev *dev)\n> \t\tPMD_INIT_LOG(ERR, \"Failed to create fdir hash table!\");\n> \t\treturn -EINVAL;\n> \t}\n> +\n> \tfdir_info->hash_map = rte_zmalloc(\"i40e_fdir_hash_map\",\n> \t\t\t\t\t sizeof(struct i40e_fdir_filter *) *\n> \t\t\t\t\t I40E_MAX_FDIR_FILTER_NUM,\n> @@ -1077,8 +1088,100 @@ i40e_init_fdir_filter_list(struct rte_eth_dev\n> *dev)\n> \t\tret = -ENOMEM;\n> \t\tgoto err_fdir_hash_map_alloc;\n> \t}\n> +\n> +\tfdir_info->fdir_filter_array = rte_zmalloc(\"fdir_filter\",\n> +\t\t\tsizeof(struct i40e_fdir_filter) *\n> +\t\t\tI40E_MAX_FDIR_FILTER_NUM,\n> +\t\t\t0);\n> +\n> +\tif (!fdir_info->fdir_filter_array) {\n> +\t\tPMD_INIT_LOG(ERR,\n> +\t\t\t \"Failed to allocate memory for fdir filter array!\");\n> +\t\tret = -ENOMEM;\n> +\t\tgoto err_fdir_filter_array_alloc;\n> +\t}\n> +\n> +\tpfqf_fdalloc = i40e_read_rx_ctl(hw, I40E_PFQF_FDALLOC);\n> +\talloc = ((pfqf_fdalloc & I40E_PFQF_FDALLOC_FDALLOC_MASK) >>\n> +\t\t\tI40E_PFQF_FDALLOC_FDALLOC_SHIFT);\n> +\tbest = ((pfqf_fdalloc & I40E_PFQF_FDALLOC_FDBEST_MASK) >>\n> +\t\t\tI40E_PFQF_FDALLOC_FDBEST_SHIFT);\n> +\n> +\tglqf_ctl_reg = i40e_read_rx_ctl(hw, I40E_GLQF_CTL);\n> +\tif (!pf->support_multi_driver) {\n> +\t\tfdir_info->fdir_invalprio = 1;\n> +\t\tglqf_ctl_reg |= I40E_GLQF_CTL_INVALPRIO_MASK;\n> +\t\tPMD_DRV_LOG(INFO, \"FDIR INVALPRIO set to guaranteed first\");\n> +\t} else {\n> +\t\tif (glqf_ctl_reg | I40E_GLQF_CTL_INVALPRIO_MASK) {\n> +\t\t\tfdir_info->fdir_invalprio = 1;\n> +\t\t\tPMD_DRV_LOG(INFO, \"FDIR INVALPRIO is: guaranteed first\");\n> +\t\t} else {\n> +\t\t\tfdir_info->fdir_invalprio = 0;\n> +\t\t\tPMD_DRV_LOG(INFO, \"FDIR INVALPRIO is: shared first\");\n> +\t\t}\n> +\t}\n> +\n> +\ti40e_write_rx_ctl(hw, I40E_GLQF_CTL, glqf_ctl_reg);\n> +\tPMD_DRV_LOG(INFO, \"FDIR guarantee space: %u, best_effort\n> space %u.\",\n> +\t\talloc * 32, best * 32);\n\nI think *32 can be applied when you assign alloc and best. \nAlso its better to replace by macro with meaningful name and use bit shift << but not *.\n> +\n> +\tfdir_info->fdir_space_size = (alloc + best) * 32;\n> +\tfdir_info->fdir_actual_cnt = 0;\n> +\tfdir_info->fdir_guarantee_available_space = alloc * 32;\n> +\tfdir_info->fdir_guarantee_free_space =\n> +\t\tfdir_info->fdir_guarantee_available_space;\n> +\n> +\tfdir_info->fdir_flow_bitmap.fdir_flow =\n> +\t\t\trte_zmalloc(\"i40e_fdir_flows\",\n> +\t\t\t\tsizeof(struct i40e_fdir_flows) *\n> +\t\t\t\tfdir_info->fdir_space_size,\n> +\t\t\t\t0);\n> +\n> +\tif (!fdir_info->fdir_flow_bitmap.fdir_flow) {\n> +\t\tPMD_INIT_LOG(ERR,\n> +\t\t\t \"Failed to allocate memory for bitmap flow!\");\n> +\t\tret = -ENOMEM;\n> +\t\tgoto err_fdir_bitmap_flow_alloc;\n> +\t}\n> +\n> +\tfor (i = 0; i < fdir_info->fdir_space_size; i++)\n> +\t\tfdir_info->fdir_flow_bitmap.fdir_flow[i].idx = i;\n> +\n> +\tbmp_size =\n> +\t\trte_bitmap_get_memory_footprint(fdir_info->fdir_space_size);\n> +\n> +\tmem = rte_zmalloc(\"fdir_bmap\", bmp_size, RTE_CACHE_LINE_SIZE);\n> +\tif (mem == NULL) {\n> +\t\tPMD_INIT_LOG(ERR,\n> +\t\t\t \"Failed to allocate memory for fdir bitmap!\");\n> +\t\tret = -ENOMEM;\n> +\t\tgoto err_fdir_mem_alloc;\n> +\t}\n> +\n> +\tbmp = rte_bitmap_init(fdir_info->fdir_space_size, mem, bmp_size);\n> +\tif (bmp == NULL) {\n> +\t\tPMD_INIT_LOG(ERR,\n> +\t\t\t \"Failed to initialization fdir bitmap!\");\n> +\t\tret = -ENOMEM;\n> +\t\tgoto err_fdir_bmp_alloc;\n> +\t}\n> +\n> +\tfor (i = 0; i < fdir_info->fdir_space_size; i++)\n> +\t\trte_bitmap_set(bmp, i);\n> +\n> +\tfdir_info->fdir_flow_bitmap.b = bmp;\n> +\n> \treturn 0;\n> \n> +err_fdir_bmp_alloc:\n> +\trte_free(mem);\n> +err_fdir_mem_alloc:\n> +\trte_free(fdir_info->fdir_flow_bitmap.fdir_flow);\n> +err_fdir_bitmap_flow_alloc:\n> +\trte_free(fdir_info->fdir_filter_array);\n> +err_fdir_filter_array_alloc:\n> +\trte_free(fdir_info->hash_map);\n> err_fdir_hash_map_alloc:\n> \trte_hash_free(fdir_info->hash_table);\n> \n> @@ -1749,18 +1852,34 @@ i40e_rm_fdir_filter_list(struct i40e_pf *pf)\n> \tstruct i40e_fdir_info *fdir_info;\n> \n> \tfdir_info = &pf->fdir;\n> -\t/* Remove all flow director rules and hash */\n> +\n> +\t/* Remove all flow director rules */\n> +\twhile ((p_fdir = TAILQ_FIRST(&fdir_info->fdir_list)))\n> +\t\tTAILQ_REMOVE(&fdir_info->fdir_list, p_fdir, rules); }\n> +\n> +static void\n> +i40e_fdir_memory_cleanup(struct i40e_pf *pf) {\n> +\tstruct i40e_fdir_info *fdir_info;\n> +\n> +\tfdir_info = &pf->fdir;\n> +\n> +\t/* flow director memory cleanup */\n> \tif (fdir_info->hash_map)\n> \t\trte_free(fdir_info->hash_map);\n> \tif (fdir_info->hash_table)\n> \t\trte_hash_free(fdir_info->hash_table);\n> +\tif (fdir_info->fdir_flow_bitmap.b)\n> +\t\trte_bitmap_free(fdir_info->fdir_flow_bitmap.b);\n> +\tif (fdir_info->fdir_flow_bitmap.fdir_flow)\n> +\t\trte_free(fdir_info->fdir_flow_bitmap.fdir_flow);\n> +\tif (fdir_info->fdir_filter_array)\n> +\t\trte_free(fdir_info->fdir_filter_array);\n> \n> -\twhile ((p_fdir = TAILQ_FIRST(&fdir_info->fdir_list))) {\n> -\t\tTAILQ_REMOVE(&fdir_info->fdir_list, p_fdir, rules);\n> -\t\trte_free(p_fdir);\n> -\t}\n> }\n> \n> +\n> void i40e_flex_payload_reg_set_default(struct i40e_hw *hw) {\n> \t/*\n> @@ -2618,9 +2737,14 @@ i40e_dev_close(struct rte_eth_dev *dev)\n> \t/* Remove all flows */\n> \twhile ((p_flow = TAILQ_FIRST(&pf->flow_list))) {\n> \t\tTAILQ_REMOVE(&pf->flow_list, p_flow, node);\n> -\t\trte_free(p_flow);\n> +\t\t/* Do not free FDIR flows since they are static allocated */\n> +\t\tif (p_flow->filter_type != RTE_ETH_FILTER_FDIR)\n> +\t\t\trte_free(p_flow);\n> \t}\n> \n> +\t/* release the fdir static allocated memory */\n> +\ti40e_fdir_memory_cleanup(pf);\n> +\n> \t/* Remove all Traffic Manager configuration */\n> \ti40e_tm_conf_uninit(dev);\n> \n> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h\n> index 31ca05de9..5861c358b 100644\n> --- a/drivers/net/i40e/i40e_ethdev.h\n> +++ b/drivers/net/i40e/i40e_ethdev.h\n> @@ -264,6 +264,15 @@ enum i40e_flxpld_layer_idx {\n> #define I40E_DEFAULT_DCB_APP_NUM 1\n> #define I40E_DEFAULT_DCB_APP_PRIO 3\n> \n> +/*\n> + * Struct to store flow created.\n> + */\n> +struct rte_flow {\n> +\tTAILQ_ENTRY(rte_flow) node;\n> +\tenum rte_filter_type filter_type;\n> +\tvoid *rule;\n> +};\n> +\n> /**\n> * The overhead from MTU to max frame size.\n> * Considering QinQ packet, the VLAN tag needs to be counted twice.\n> @@ -674,17 +683,33 @@ struct i40e_fdir_filter {\n> \tstruct i40e_fdir_filter_conf fdir;\n> };\n> \n> +struct i40e_fdir_flows {\n\nWhy it be named as \"flows\"\nCould you add more comment here, what's purpose of the data structure and each field?\n\n> +\tuint32_t idx;\n> +\tstruct rte_flow flow;\n\nNot sure if its better to move flow to the first field, so we cover between a rte_flow point and a i40e_fdir_flow point directly.\n> +};\n> +\n> +struct i40e_fdir_flow_bitmap {\n> +\tstruct rte_bitmap *b;\n> +\tstruct i40e_fdir_flows *fdir_flow;\n> +};\n\nCan we just add rte_bitmap into i40e_fdir_flow?\n\n> +\n> +#define FLOW_TO_FLOW_BITMAP(f) \\\n> +\tcontainer_of((f), struct i40e_fdir_flows, flow)\n> +\n> TAILQ_HEAD(i40e_fdir_filter_list, i40e_fdir_filter);\n> /*\n> * A structure used to define fields of a FDIR related info.\n> */\n> struct i40e_fdir_info {\n> +#define PRG_PKT_CNT\t128\n> +\n> \tstruct i40e_vsi *fdir_vsi; /* pointer to fdir VSI structure */\n> \tuint16_t match_counter_index; /* Statistic counter index used for\n> fdir*/\n> \tstruct i40e_tx_queue *txq;\n> \tstruct i40e_rx_queue *rxq;\n> -\tvoid *prg_pkt; /* memory for fdir program packet */\n> -\tuint64_t dma_addr; /* physic address of packet\n> memory*/\n> +\tvoid *prg_pkt[PRG_PKT_CNT]; /* memory for fdir program packet\n> */\n> +\tuint64_t dma_addr[PRG_PKT_CNT];\t/* physic address of packet\n> memory*/\n> +\n> \t/* input set bits for each pctype */\n> \tuint64_t input_set[I40E_FILTER_PCTYPE_MAX];\n> \t/*\n> @@ -698,6 +723,27 @@ struct i40e_fdir_info {\n> \tstruct i40e_fdir_filter **hash_map;\n> \tstruct rte_hash *hash_table;\n> \n> +\tstruct i40e_fdir_filter *fdir_filter_array;\n> +\n> +\t/*\n> +\t * Priority ordering at filter invalidation(destroying a flow) between\n> +\t * \"best effort\" space and \"guaranteed\" space.\n> +\t *\n> +\t * 0 = At filter invalidation, the hardware first tries to increment the\n> +\t * \"best effort\" space. The \"guaranteed\" space is incremented only\n> when\n> +\t * the global \"best effort\" space is at it max value or the \"best effort\"\n> +\t * space of the PF is at its max value.\n> +\t * 1 = At filter invalidation, the hardware first tries to increment its\n> +\t * \"guaranteed\" space. The \"best effort\" space is incremented only\n> when\n> +\t * it is already at its max value.\n> +\t */\n> +\tuint32_t fdir_invalprio;\n> +\tuint32_t fdir_space_size;\n> +\tuint32_t fdir_actual_cnt;\n> +\tuint32_t fdir_guarantee_available_space;\n> +\tuint32_t fdir_guarantee_free_space;\n> +\tstruct i40e_fdir_flow_bitmap fdir_flow_bitmap;\n\nWhat is the flow_bitmap usage? its free bitmap or alloc bitmap?\nBetter add more description here, or rename it with more clean purpose.\n\n> +\n> \t/* Mark if flex pit and mask is set */\n> \tbool flex_pit_flag[I40E_MAX_FLXPLD_LAYER];\n> \tbool flex_mask_flag[I40E_FILTER_PCTYPE_MAX];\n> @@ -894,15 +940,6 @@ struct i40e_mirror_rule {\n> \n> TAILQ_HEAD(i40e_mirror_rule_list, i40e_mirror_rule);\n> \n> -/*\n> - * Struct to store flow created.\n> - */\n> -struct rte_flow {\n> -\tTAILQ_ENTRY(rte_flow) node;\n> -\tenum rte_filter_type filter_type;\n> -\tvoid *rule;\n> -};\n> -\n> TAILQ_HEAD(i40e_flow_list, rte_flow);\n> \n> /* Struct to store Traffic Manager shaper profile. */ @@ -1335,8 +1372,8\n> @@ int i40e_add_del_fdir_filter(struct rte_eth_dev *dev,\n> \t\t\t const struct rte_eth_fdir_filter *filter,\n> \t\t\t bool add);\n> int i40e_flow_add_del_fdir_filter(struct rte_eth_dev *dev,\n> -\t\t\t\t const struct i40e_fdir_filter_conf *filter,\n> -\t\t\t\t bool add);\n> +\t\t\t const struct i40e_fdir_filter_conf *filter,\n> +\t\t\t bool add, bool wait_status);\n> int i40e_dev_tunnel_filter_set(struct i40e_pf *pf,\n> \t\t\t struct rte_eth_tunnel_filter_conf *tunnel_filter,\n> \t\t\t uint8_t add);\n> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c index\n> 4a778db4c..d7ba841d6 100644\n> --- a/drivers/net/i40e/i40e_fdir.c\n> +++ b/drivers/net/i40e/i40e_fdir.c\n> @@ -99,7 +99,7 @@ static int\n> i40e_flow_fdir_filter_programming(struct i40e_pf *pf,\n> \t\t\t\t enum i40e_filter_pctype pctype,\n> \t\t\t\t const struct i40e_fdir_filter_conf *filter,\n> -\t\t\t\t bool add);\n> +\t\t\t\t bool add, bool wait_status);\n> \n> static int\n> i40e_fdir_rx_queue_init(struct i40e_rx_queue *rxq) @@ -163,6 +163,7\n> @@ i40e_fdir_setup(struct i40e_pf *pf)\n> \tchar z_name[RTE_MEMZONE_NAMESIZE];\n> \tconst struct rte_memzone *mz = NULL;\n> \tstruct rte_eth_dev *eth_dev = pf->adapter->eth_dev;\n> +\tuint16_t i;\n> \n> \tif ((pf->flags & I40E_FLAG_FDIR) == 0) {\n> \t\tPMD_INIT_LOG(ERR, \"HW doesn't support FDIR\"); @@ -179,6\n> +180,7 @@ i40e_fdir_setup(struct i40e_pf *pf)\n> \t\tPMD_DRV_LOG(INFO, \"FDIR initialization has been done.\");\n> \t\treturn I40E_SUCCESS;\n> \t}\n> +\n> \t/* make new FDIR VSI */\n> \tvsi = i40e_vsi_setup(pf, I40E_VSI_FDIR, pf->main_vsi, 0);\n> \tif (!vsi) {\n> @@ -233,17 +235,27 @@ i40e_fdir_setup(struct i40e_pf *pf)\n> \t\t\teth_dev->device->driver->name,\n> \t\t\tI40E_FDIR_MZ_NAME,\n> \t\t\teth_dev->data->port_id);\n> -\tmz = i40e_memzone_reserve(z_name, I40E_FDIR_PKT_LEN,\n> SOCKET_ID_ANY);\n> +\tmz = i40e_memzone_reserve(z_name, I40E_FDIR_PKT_LEN *\n> PRG_PKT_CNT,\n> +\t\t\tSOCKET_ID_ANY);\n> \tif (!mz) {\n> \t\tPMD_DRV_LOG(ERR, \"Cannot init memzone for \"\n> \t\t\t\t \"flow director program packet.\");\n> \t\terr = I40E_ERR_NO_MEMORY;\n> \t\tgoto fail_mem;\n> \t}\n> -\tpf->fdir.prg_pkt = mz->addr;\n> -\tpf->fdir.dma_addr = mz->iova;\n> +\n> +\tfor (i = 0; i < PRG_PKT_CNT; i++) {\n> +\t\tpf->fdir.prg_pkt[i] = (uint8_t *)mz->addr + I40E_FDIR_PKT_LEN *\n> +\t\t\t\t(i % PRG_PKT_CNT);\n\nThe loop is from 0 to PRG_PKT_CNT, why \"i % PKG_PTK_CNT\"?\n\n> +\t\tpf->fdir.dma_addr[i] = mz->iova + I40E_FDIR_PKT_LEN *\n> +\t\t\t\t(i % PRG_PKT_CNT);\n> +\t}\n> \n> \tpf->fdir.match_counter_index =\n> I40E_COUNTER_INDEX_FDIR(hw->pf_id);\n> +\tpf->fdir.fdir_actual_cnt = 0;\n> +\tpf->fdir.fdir_guarantee_free_space =\n> +\t\tpf->fdir.fdir_guarantee_available_space;\n> +\n> \tPMD_DRV_LOG(INFO, \"FDIR setup successfully, with programming\n> queue %u.\",\n> \t\t vsi->base_queue);\n> \treturn I40E_SUCCESS;\n> @@ -327,6 +339,7 @@ i40e_init_flx_pld(struct i40e_pf *pf)\n> \t\tpf->fdir.flex_set[index].src_offset = 0;\n> \t\tpf->fdir.flex_set[index].size = I40E_FDIR_MAX_FLEXWORD_NUM;\n> \t\tpf->fdir.flex_set[index].dst_offset = 0;\n> +\nDummy empty line.\n\n> \t\tI40E_WRITE_REG(hw, I40E_PRTQF_FLX_PIT(index), 0x0000C900);\n> \t\tI40E_WRITE_REG(hw,\n> \t\t\tI40E_PRTQF_FLX_PIT(index + 1), 0x0000FC29);/*non-used*/\n> @@ -1557,11 +1570,11 @@ i40e_sw_fdir_filter_lookup(struct\n> i40e_fdir_info *fdir_info,\n> \treturn fdir_info->hash_map[ret];\n> }\n> \n> -/* Add a flow director filter into the SW list */ static int\n> i40e_sw_fdir_filter_insert(struct i40e_pf *pf, struct i40e_fdir_filter *filter)\n> {\n> \tstruct i40e_fdir_info *fdir_info = &pf->fdir;\n> +\tstruct i40e_fdir_filter *hash_filter;\n> \tint ret;\n> \n> \tif (filter->fdir.input.flow_ext.pkt_template)\n> @@ -1577,9 +1590,14 @@ i40e_sw_fdir_filter_insert(struct i40e_pf *pf,\n> struct i40e_fdir_filter *filter)\n> \t\t\t ret);\n> \t\treturn ret;\n> \t}\n> -\tfdir_info->hash_map[ret] = filter;\n> \n> -\tTAILQ_INSERT_TAIL(&fdir_info->fdir_list, filter, rules);\n> +\tif (fdir_info->hash_map[ret])\n> +\t\treturn -1;\n> +\n> +\thash_filter = &fdir_info->fdir_filter_array[ret];\n> +\trte_memcpy(hash_filter, filter, sizeof(*filter));\n> +\tfdir_info->hash_map[ret] = hash_filter;\n> +\tTAILQ_INSERT_TAIL(&fdir_info->fdir_list, hash_filter, rules);\n> \n> \treturn 0;\n> }\n> @@ -1608,7 +1626,6 @@ i40e_sw_fdir_filter_del(struct i40e_pf *pf, struct\n> i40e_fdir_input *input)\n> \tfdir_info->hash_map[ret] = NULL;\n> \n> \tTAILQ_REMOVE(&fdir_info->fdir_list, filter, rules);\n> -\trte_free(filter);\n> \n> \treturn 0;\n> }\n> @@ -1675,23 +1692,50 @@ i40e_add_del_fdir_filter(struct rte_eth_dev\n> *dev,\n> \treturn ret;\n> }\n> \n> +static inline unsigned char *\n> +i40e_find_available_buffer(struct rte_eth_dev *dev) {\n> +\tstruct i40e_pf *pf =\n> I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);\n> +\tstruct i40e_fdir_info *fdir_info = &pf->fdir;\n> +\tstruct i40e_tx_queue *txq = pf->fdir.txq;\n> +\tvolatile struct i40e_tx_desc *txdp = &txq->tx_ring[txq->tx_tail + 1];\n> +\tuint32_t i;\n> +\n> +\t/* wait until the tx descriptor is ready */\n> +\tfor (i = 0; i < I40E_FDIR_MAX_WAIT_US; i++) {\n> +\t\tif ((txdp->cmd_type_offset_bsz &\n> +\t\t\trte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==\n> +\t\t\trte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))\n> +\t\t\tbreak;\n> +\t\trte_delay_us(1);\n> +\t}\n> +\tif (i >= I40E_FDIR_MAX_WAIT_US) {\n> +\t\tPMD_DRV_LOG(ERR,\n> +\t\t \"Failed to program FDIR filter: time out to get DD on tx\n> queue.\");\n> +\t\treturn NULL;\n> +\t}\n> +\n> +\treturn (unsigned char *)fdir_info->prg_pkt[txq->tx_tail / 2]; }\n\nwhy tx_tail / 2, better some add some description here, and use \" >> 1\" if its word / byte conversion.\n\n> +\n> /**\n> * i40e_flow_add_del_fdir_filter - add or remove a flow director filter.\n> * @pf: board private structure\n> * @filter: fdir filter entry\n> * @add: 0 - delete, 1 - add\n> */\n> +\n> int\n> i40e_flow_add_del_fdir_filter(struct rte_eth_dev *dev,\n> \t\t\t const struct i40e_fdir_filter_conf *filter,\n> -\t\t\t bool add)\n> +\t\t\t bool add, bool wait_status)\n> {\n> \tstruct i40e_hw *hw =\n> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);\n> \tstruct i40e_pf *pf =\n> I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);\n> -\tunsigned char *pkt = (unsigned char *)pf->fdir.prg_pkt;\n> +\tunsigned char *pkt = NULL;\n> \tenum i40e_filter_pctype pctype;\n> \tstruct i40e_fdir_info *fdir_info = &pf->fdir;\n> -\tstruct i40e_fdir_filter *fdir_filter, *node;\n> +\tstruct i40e_fdir_filter *node;\n> \tstruct i40e_fdir_filter check_filter; /* Check if the filter exists */\n> \tint ret = 0;\n> \n> @@ -1724,25 +1768,41 @@ i40e_flow_add_del_fdir_filter(struct\n> rte_eth_dev *dev,\n> \t/* Check if there is the filter in SW list */\n> \tmemset(&check_filter, 0, sizeof(check_filter));\n> \ti40e_fdir_filter_convert(filter, &check_filter);\n> -\tnode = i40e_sw_fdir_filter_lookup(fdir_info, &check_filter.fdir.input);\n> -\tif (add && node) {\n> -\t\tPMD_DRV_LOG(ERR,\n> -\t\t\t \"Conflict with existing flow director rules!\");\n> -\t\treturn -EINVAL;\n> -\t}\n> \n> -\tif (!add && !node) {\n> -\t\tPMD_DRV_LOG(ERR,\n> -\t\t\t \"There's no corresponding flow firector filter!\");\n> -\t\treturn -EINVAL;\n> +\tif (add) {\n> +\t\tret = i40e_sw_fdir_filter_insert(pf, &check_filter);\n> +\t\tif (ret < 0) {\n> +\t\t\tPMD_DRV_LOG(ERR,\n> +\t\t\t\t \"Conflict with existing flow director rules!\");\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\t} else {\n> +\t\tnode = i40e_sw_fdir_filter_lookup(fdir_info,\n> +\t\t\t\t&check_filter.fdir.input);\n> +\t\tif (!node) {\n> +\t\t\tPMD_DRV_LOG(ERR,\n> +\t\t\t\t \"There's no corresponding flow firector filter!\");\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\tret = i40e_sw_fdir_filter_del(pf, &node->fdir.input);\n> +\t\tif (ret < 0) {\n> +\t\t\tPMD_DRV_LOG(ERR,\n> +\t\t\t\t\t\"Error deleting fdir rule from hash table!\");\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> \t}\n> \n> -\tmemset(pkt, 0, I40E_FDIR_PKT_LEN);\n> +\t/* find a buffer to store the pkt */\n> +\tpkt = i40e_find_available_buffer(dev);\n> +\tif (pkt == NULL)\n> +\t\tgoto error_op;\n> \n> +\tmemset(pkt, 0, I40E_FDIR_PKT_LEN);\n> \tret = i40e_flow_fdir_construct_pkt(pf, &filter->input, pkt);\n> \tif (ret < 0) {\n> \t\tPMD_DRV_LOG(ERR, \"construct packet for fdir fails.\");\n> -\t\treturn ret;\n> +\t\tgoto error_op;\n> \t}\n> \n> \tif (hw->mac.type == I40E_MAC_X722) {\n> @@ -1751,28 +1811,22 @@ i40e_flow_add_del_fdir_filter(struct\n> rte_eth_dev *dev,\n> \t\t\thw, I40E_GLQF_FD_PCTYPES((int)pctype));\n> \t}\n> \n> -\tret = i40e_flow_fdir_filter_programming(pf, pctype, filter, add);\n> +\tret = i40e_flow_fdir_filter_programming(pf, pctype, filter, add,\n> +\t\t\twait_status);\n> \tif (ret < 0) {\n> \t\tPMD_DRV_LOG(ERR, \"fdir programming fails for PCTYPE(%u).\",\n> \t\t\t pctype);\n> -\t\treturn ret;\n> +\t\tgoto error_op;\n> \t}\n> \n> -\tif (add) {\n> -\t\tfdir_filter = rte_zmalloc(\"fdir_filter\",\n> -\t\t\t\t\t sizeof(*fdir_filter), 0);\n> -\t\tif (fdir_filter == NULL) {\n> -\t\t\tPMD_DRV_LOG(ERR, \"Failed to alloc memory.\");\n> -\t\t\treturn -ENOMEM;\n> -\t\t}\n> +\treturn ret;\n> \n> -\t\trte_memcpy(fdir_filter, &check_filter, sizeof(check_filter));\n> -\t\tret = i40e_sw_fdir_filter_insert(pf, fdir_filter);\n> -\t\tif (ret < 0)\n> -\t\t\trte_free(fdir_filter);\n> -\t} else {\n> -\t\tret = i40e_sw_fdir_filter_del(pf, &node->fdir.input);\n> -\t}\n> +error_op:\n> +\t/* roll back */\n> +\tif (add)\n> +\t\ti40e_sw_fdir_filter_del(pf, &check_filter.fdir.input);\n> +\telse\n> +\t\ti40e_sw_fdir_filter_insert(pf, &check_filter);\n> \n> \treturn ret;\n> }\n> @@ -1875,7 +1929,7 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,\n> \n> \tPMD_DRV_LOG(INFO, \"filling transmit descriptor.\");\n> \ttxdp = &(txq->tx_ring[txq->tx_tail + 1]);\n> -\ttxdp->buffer_addr = rte_cpu_to_le_64(pf->fdir.dma_addr);\n> +\ttxdp->buffer_addr = rte_cpu_to_le_64(pf->fdir.dma_addr[0]);\n> \ttd_cmd = I40E_TX_DESC_CMD_EOP |\n> \t\t I40E_TX_DESC_CMD_RS |\n> \t\t I40E_TX_DESC_CMD_DUMMY;\n> @@ -1925,7 +1979,7 @@ static int\n> i40e_flow_fdir_filter_programming(struct i40e_pf *pf,\n> \t\t\t\t enum i40e_filter_pctype pctype,\n> \t\t\t\t const struct i40e_fdir_filter_conf *filter,\n> -\t\t\t\t bool add)\n> +\t\t\t\t bool add, bool wait_status)\n> {\n> \tstruct i40e_tx_queue *txq = pf->fdir.txq;\n> \tstruct i40e_rx_queue *rxq = pf->fdir.rxq; @@ -1933,8 +1987,10 @@\n> i40e_flow_fdir_filter_programming(struct i40e_pf *pf,\n> \tvolatile struct i40e_tx_desc *txdp;\n> \tvolatile struct i40e_filter_program_desc *fdirdp;\n> \tuint32_t td_cmd;\n> -\tuint16_t vsi_id, i;\n> +\tuint16_t vsi_id;\n> \tuint8_t dest;\n> +\tuint32_t i;\n> +\tuint8_t retry_count = 0;\n> \n> \tPMD_DRV_LOG(INFO, \"filling filter programming descriptor.\");\n> \tfdirdp = (volatile struct i40e_filter_program_desc *) @@ -2009,7\n> +2065,8 @@ i40e_flow_fdir_filter_programming(struct i40e_pf *pf,\n> \n> \tPMD_DRV_LOG(INFO, \"filling transmit descriptor.\");\n> \ttxdp = &txq->tx_ring[txq->tx_tail + 1];\n> -\ttxdp->buffer_addr = rte_cpu_to_le_64(pf->fdir.dma_addr);\n> +\ttxdp->buffer_addr = rte_cpu_to_le_64(pf->fdir.dma_addr[txq->tx_tail /\n> +2]);\n> +\n> \ttd_cmd = I40E_TX_DESC_CMD_EOP |\n> \t\t I40E_TX_DESC_CMD_RS |\n> \t\t I40E_TX_DESC_CMD_DUMMY;\n> @@ -2022,25 +2079,34 @@ i40e_flow_fdir_filter_programming(struct\n> i40e_pf *pf,\n> \t\ttxq->tx_tail = 0;\n> \t/* Update the tx tail register */\n> \trte_wmb();\n> +\n> +\t/* capture the previous error report(if any) from rx ring */\n> +\twhile ((i40e_check_fdir_programming_status(rxq) < 0) &&\n> +\t\t(++retry_count < 100))\n> +\t\tPMD_DRV_LOG(INFO, \"previous error report captured.\");\n> +\n> \tI40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);\n> -\tfor (i = 0; i < I40E_FDIR_MAX_WAIT_US; i++) {\n> -\t\tif ((txdp->cmd_type_offset_bsz &\n> -\t\t\t\trte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==\n> -\t\t\t\trte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))\n> -\t\t\tbreak;\n> -\t\trte_delay_us(1);\n> -\t}\n> -\tif (i >= I40E_FDIR_MAX_WAIT_US) {\n> -\t\tPMD_DRV_LOG(ERR,\n> -\t\t \"Failed to program FDIR filter: time out to get DD on tx\n> queue.\");\n> -\t\treturn -ETIMEDOUT;\n> -\t}\n> -\t/* totally delay 10 ms to check programming status*/\n> -\trte_delay_us(I40E_FDIR_MAX_WAIT_US);\n> -\tif (i40e_check_fdir_programming_status(rxq) < 0) {\n> -\t\tPMD_DRV_LOG(ERR,\n> -\t\t \"Failed to program FDIR filter: programming status\n> reported.\");\n> -\t\treturn -ETIMEDOUT;\n> +\n> +\tif (wait_status) {\n> +\t\tfor (i = 0; i < I40E_FDIR_MAX_WAIT_US; i++) {\n> +\t\t\tif ((txdp->cmd_type_offset_bsz &\n> +\t\t\t\t\trte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK))\n> ==\n> +\n> \trte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))\n> +\t\t\t\tbreak;\n> +\t\t\trte_delay_us(1);\n> +\t\t}\n> +\t\tif (i >= I40E_FDIR_MAX_WAIT_US) {\n> +\t\t\tPMD_DRV_LOG(ERR,\n> +\t\t\t \"Failed to program FDIR filter: time out to get DD on tx\n> queue.\");\n> +\t\t\treturn -ETIMEDOUT;\n> +\t\t}\n> +\t\t/* totally delay 10 ms to check programming status*/\n> +\t\trte_delay_us(I40E_FDIR_MAX_WAIT_US);\n> +\t\tif (i40e_check_fdir_programming_status(rxq) < 0) {\n> +\t\t\tPMD_DRV_LOG(ERR,\n> +\t\t\t \"Failed to program FDIR filter: programming status\n> reported.\");\n> +\t\t\treturn -ETIMEDOUT;\n> +\t\t}\n> \t}\n> \n> \treturn 0;\n> @@ -2324,7 +2390,7 @@ i40e_fdir_filter_restore(struct i40e_pf *pf)\n> \tuint32_t best_cnt; /**< Number of filters in best effort spaces. */\n> \n> \tTAILQ_FOREACH(f, fdir_list, rules)\n> -\t\ti40e_flow_add_del_fdir_filter(dev, &f->fdir, TRUE);\n> +\t\ti40e_flow_add_del_fdir_filter(dev, &f->fdir, TRUE, TRUE);\n> \n> \tfdstat = I40E_READ_REG(hw, I40E_PFQF_FDSTAT);\n> \tguarant_cnt =\n> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c\n> index 7cd537340..260e58dc1 100644\n> --- a/drivers/net/i40e/i40e_flow.c\n> +++ b/drivers/net/i40e/i40e_flow.c\n> @@ -17,6 +17,7 @@\n> #include <rte_malloc.h>\n> #include <rte_tailq.h>\n> #include <rte_flow_driver.h>\n> +#include <rte_bitmap.h>\n> \n> #include \"i40e_logs.h\"\n> #include \"base/i40e_type.h\"\n> @@ -144,6 +145,8 @@ const struct rte_flow_ops i40e_flow_ops = {\n> \n> static union i40e_filter_t cons_filter; static enum rte_filter_type\n> cons_filter_type = RTE_ETH_FILTER_NONE;\n> +/* internal pattern w/o VOID items */\n> +struct rte_flow_item g_items[32];\n> \n> /* Pattern matched ethertype filter */\n> static enum rte_flow_item_type pattern_ethertype[] = { @@ -2044,9\n> +2047,6 @@ i40e_flow_parse_ethertype_pattern(struct rte_eth_dev *dev,\n> \tconst struct rte_flow_item_eth *eth_spec;\n> \tconst struct rte_flow_item_eth *eth_mask;\n> \tenum rte_flow_item_type item_type;\n> -\tuint16_t outer_tpid;\n> -\n> -\touter_tpid = i40e_get_outer_vlan(dev);\n> \n> \tfor (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {\n> \t\tif (item->last) {\n> @@ -2106,7 +2106,7 @@ i40e_flow_parse_ethertype_pattern(struct\n> rte_eth_dev *dev,\n> \t\t\tif (filter->ether_type == RTE_ETHER_TYPE_IPV4 ||\n> \t\t\t filter->ether_type == RTE_ETHER_TYPE_IPV6 ||\n> \t\t\t filter->ether_type == RTE_ETHER_TYPE_LLDP ||\n> -\t\t\t filter->ether_type == outer_tpid) {\n> +\t\t\t filter->ether_type == i40e_get_outer_vlan(dev)) {\n> \t\t\t\trte_flow_error_set(error, EINVAL,\n> \t\t\t\t\t\t RTE_FLOW_ERROR_TYPE_ITEM,\n> \t\t\t\t\t\t item,\n> @@ -2349,6 +2349,7 @@ i40e_flow_set_fdir_flex_pit(struct i40e_pf *pf,\n> \t\tfield_idx = layer_idx * I40E_MAX_FLXPLD_FIED + i;\n> \t\tflx_pit = MK_FLX_PIT(min_next_off, NONUSE_FLX_PIT_FSIZE,\n> \t\t\t\t NONUSE_FLX_PIT_DEST_OFF);\n> +\n> \t\tI40E_WRITE_REG(hw, I40E_PRTQF_FLX_PIT(field_idx), flx_pit);\n> \t\tmin_next_off++;\n> \t}\n> @@ -2608,7 +2609,6 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev\n> *dev,\n> \tuint16_t flex_size;\n> \tbool cfg_flex_pit = true;\n> \tbool cfg_flex_msk = true;\n> -\tuint16_t outer_tpid;\n> \tuint16_t ether_type;\n> \tuint32_t vtc_flow_cpu;\n> \tbool outer_ip = true;\n> @@ -2617,7 +2617,6 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev\n> *dev,\n> \tmemset(off_arr, 0, sizeof(off_arr));\n> \tmemset(len_arr, 0, sizeof(len_arr));\n> \tmemset(flex_mask, 0, I40E_FDIR_MAX_FLEX_LEN);\n> -\touter_tpid = i40e_get_outer_vlan(dev);\n> \tfilter->input.flow_ext.customized_pctype = false;\n> \tfor (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {\n> \t\tif (item->last) {\n> @@ -2685,7 +2684,7 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev\n> *dev,\n> \t\t\t\tif (next_type == RTE_FLOW_ITEM_TYPE_VLAN ||\n> \t\t\t\t ether_type == RTE_ETHER_TYPE_IPV4 ||\n> \t\t\t\t ether_type == RTE_ETHER_TYPE_IPV6 ||\n> -\t\t\t\t ether_type == outer_tpid) {\n> +\t\t\t\t ether_type == i40e_get_outer_vlan(dev)) {\n> \t\t\t\t\trte_flow_error_set(error, EINVAL,\n> \t\t\t\t\t\t RTE_FLOW_ERROR_TYPE_ITEM,\n> \t\t\t\t\t\t item,\n> @@ -2729,7 +2728,7 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev\n> *dev,\n> \n> \t\t\t\tif (ether_type == RTE_ETHER_TYPE_IPV4 ||\n> \t\t\t\t ether_type == RTE_ETHER_TYPE_IPV6 ||\n> -\t\t\t\t ether_type == outer_tpid) {\n> +\t\t\t\t ether_type == i40e_get_outer_vlan(dev)) {\n> \t\t\t\t\trte_flow_error_set(error, EINVAL,\n> \t\t\t\t\t\t RTE_FLOW_ERROR_TYPE_ITEM,\n> \t\t\t\t\t\t item,\n> @@ -5263,7 +5262,6 @@ i40e_flow_validate(struct rte_eth_dev *dev,\n> \t\t\t\t NULL, \"NULL attribute.\");\n> \t\treturn -rte_errno;\n> \t}\n> -\n> \tmemset(&cons_filter, 0, sizeof(cons_filter));\n> \n> \t/* Get the non-void item of action */\n> @@ -5285,12 +5283,18 @@ i40e_flow_validate(struct rte_eth_dev *dev,\n> \t}\n> \titem_num++;\n> \n> -\titems = rte_zmalloc(\"i40e_pattern\",\n> -\t\t\t item_num * sizeof(struct rte_flow_item), 0);\n> -\tif (!items) {\n> -\t\trte_flow_error_set(error, ENOMEM,\n> RTE_FLOW_ERROR_TYPE_ITEM_NUM,\n> -\t\t\t\t NULL, \"No memory for PMD internal items.\");\n> -\t\treturn -ENOMEM;\n> +\tif (item_num <= ARRAY_SIZE(g_items)) {\n> +\t\titems = g_items;\n> +\t} else {\n> +\t\titems = rte_zmalloc(\"i40e_pattern\",\n> +\t\t\t\t item_num * sizeof(struct rte_flow_item), 0);\n> +\t\tif (!items) {\n> +\t\t\trte_flow_error_set(error, ENOMEM,\n> +\t\t\t\t\tRTE_FLOW_ERROR_TYPE_ITEM_NUM,\n> +\t\t\t\t\tNULL,\n> +\t\t\t\t\t\"No memory for PMD internal items.\");\n> +\t\t\treturn -ENOMEM;\n> +\t\t}\n> \t}\n> \n> \ti40e_pattern_skip_void_item(items, pattern); @@ -5298,20 +5302,26\n> @@ i40e_flow_validate(struct rte_eth_dev *dev,\n> \ti = 0;\n> \tdo {\n> \t\tparse_filter = i40e_find_parse_filter_func(items, &i);\n> +\n> \t\tif (!parse_filter && !flag) {\n> \t\t\trte_flow_error_set(error, EINVAL,\n> \t\t\t\t\t RTE_FLOW_ERROR_TYPE_ITEM,\n> \t\t\t\t\t pattern, \"Unsupported pattern\");\n> -\t\t\trte_free(items);\n> +\n> +\t\t\tif (items != g_items)\n> +\t\t\t\trte_free(items);\n> \t\t\treturn -rte_errno;\n> \t\t}\n> +\n> \t\tif (parse_filter)\n> \t\t\tret = parse_filter(dev, attr, items, actions,\n> \t\t\t\t\t error, &cons_filter);\n> +\n> \t\tflag = true;\n> \t} while ((ret < 0) && (i < RTE_DIM(i40e_supported_patterns)));\n> \n> -\trte_free(items);\n> +\tif (items != g_items)\n> +\t\trte_free(items);\n> \n> \treturn ret;\n> }\n> @@ -5324,21 +5334,67 @@ i40e_flow_create(struct rte_eth_dev *dev,\n> \t\t struct rte_flow_error *error)\n> {\n> \tstruct i40e_pf *pf =\n> I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);\n> -\tstruct rte_flow *flow;\n> +\tstruct rte_flow *flow = NULL;\n> +\tstruct i40e_fdir_info *fdir_info = &pf->fdir;\n> +\tuint32_t i = 0;\n> +\tuint32_t pos = 0;\n> +\tuint64_t slab = 0;\n> +\tbool wait_for_status = true;\n> \tint ret;\n> \n> -\tflow = rte_zmalloc(\"i40e_flow\", sizeof(struct rte_flow), 0);\n> -\tif (!flow) {\n> -\t\trte_flow_error_set(error, ENOMEM,\n> -\t\t\t\t RTE_FLOW_ERROR_TYPE_HANDLE, NULL,\n> -\t\t\t\t \"Failed to allocate memory\");\n> -\t\treturn flow;\n> -\t}\n> -\n> \tret = i40e_flow_validate(dev, attr, pattern, actions, error);\n> \tif (ret < 0)\n> \t\treturn NULL;\n> \n> +\tif (cons_filter_type == RTE_ETH_FILTER_FDIR) {\n> +\t\tif (fdir_info->fdir_actual_cnt >=\n> +\t\t\t\tfdir_info->fdir_space_size) {\n> +\t\t\trte_flow_error_set(error, ENOMEM,\n> +\t\t\t\t\t RTE_FLOW_ERROR_TYPE_HANDLE, NULL,\n> +\t\t\t\t\t \"Fdir space full\");\n> +\t\t\treturn NULL;\n> +\t\t}\n> +\n> +\t\tret = rte_bitmap_scan(fdir_info->fdir_flow_bitmap.b, &pos,\n> +\t\t\t\t&slab);\n> +\n> +\t\t/* normally this won't happen as the fdir_actual_cnt should be\n> +\t\t * same with the number of the set bits in fdir_flow_bitmap,\n> +\t\t * but anyway handle this error condition here for safe\n> +\t\t */\n> +\t\tif (ret == 0) {\n> +\t\t\tPMD_DRV_LOG(ERR, \"fdir_actual_cnt out of sync\");\n> +\t\t\trte_flow_error_set(error, ENOMEM,\n> +\t\t\t\t\t RTE_FLOW_ERROR_TYPE_HANDLE, NULL,\n> +\t\t\t\t\t \"Fdir space full\");\n> +\t\t\treturn NULL;\n> +\t\t}\n> +\n> +\t\ti = rte_bsf64(slab);\n> +\t\tpos += i;\n> +\t\trte_bitmap_clear(fdir_info->fdir_flow_bitmap.b, pos);\n> +\t\tflow = &fdir_info->fdir_flow_bitmap.fdir_flow[pos].flow;\n> +\t\tmemset(flow, 0, sizeof(struct rte_flow));\n> +\n> +\t\tif (fdir_info->fdir_invalprio == 1) {\n> +\t\t\tif (fdir_info->fdir_guarantee_free_space > 0) {\n> +\t\t\t\tfdir_info->fdir_guarantee_free_space--;\n> +\t\t\t\twait_for_status = false;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tfdir_info->fdir_actual_cnt++;\n> +\n> +\t} else {\n> +\t\tflow = rte_zmalloc(\"i40e_flow\", sizeof(struct rte_flow), 0);\n> +\t\tif (!flow) {\n> +\t\t\trte_flow_error_set(error, ENOMEM,\n> +\t\t\t\t\t RTE_FLOW_ERROR_TYPE_HANDLE, NULL,\n> +\t\t\t\t\t \"Failed to allocate memory\");\n> +\t\t\treturn flow;\n> +\t\t}\n> +\t}\n> +\n> \tswitch (cons_filter_type) {\n> \tcase RTE_ETH_FILTER_ETHERTYPE:\n> \t\tret = i40e_ethertype_filter_set(pf,\n> @@ -5350,9 +5406,17 @@ i40e_flow_create(struct rte_eth_dev *dev,\n> \t\tbreak;\n> \tcase RTE_ETH_FILTER_FDIR:\n> \t\tret = i40e_flow_add_del_fdir_filter(dev,\n> -\t\t\t\t &cons_filter.fdir_filter, 1);\n> -\t\tif (ret)\n> +\t\t\t &cons_filter.fdir_filter, 1,\n> +\t\t\t wait_for_status);\n> +\t\tif (ret) {\n> +\t\t\trte_bitmap_set(fdir_info->fdir_flow_bitmap.b, pos);\n> +\t\t\tfdir_info->fdir_actual_cnt--;\n> +\t\t\tif (fdir_info->fdir_invalprio == 1)\n> +\t\t\t\tfdir_info->fdir_guarantee_free_space++;\n> +\n> \t\t\tgoto free_flow;\n> +\t\t}\n> +\n> \t\tflow->rule = TAILQ_LAST(&pf->fdir.fdir_list,\n> \t\t\t\t\ti40e_fdir_filter_list);\n> \t\tbreak;\n> @@ -5384,7 +5448,10 @@ i40e_flow_create(struct rte_eth_dev *dev,\n> \trte_flow_error_set(error, -ret,\n> \t\t\t RTE_FLOW_ERROR_TYPE_HANDLE, NULL,\n> \t\t\t \"Failed to create flow.\");\n> -\trte_free(flow);\n> +\n> +\tif (cons_filter_type != RTE_ETH_FILTER_FDIR)\n> +\t\trte_free(flow);\n> +\n> \treturn NULL;\n> }\n> \n> @@ -5394,7 +5461,9 @@ i40e_flow_destroy(struct rte_eth_dev *dev,\n> \t\t struct rte_flow_error *error)\n> {\n> \tstruct i40e_pf *pf =\n> I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);\n> +\tstruct i40e_fdir_info *fdir_info = &pf->fdir;\n> \tenum rte_filter_type filter_type = flow->filter_type;\n> +\tstruct i40e_fdir_flows *f;\n> \tint ret = 0;\n> \n> \tswitch (filter_type) {\n> @@ -5408,7 +5477,8 @@ i40e_flow_destroy(struct rte_eth_dev *dev,\n> \t\tbreak;\n> \tcase RTE_ETH_FILTER_FDIR:\n> \t\tret = i40e_flow_add_del_fdir_filter(dev,\n> -\t\t &((struct i40e_fdir_filter *)flow->rule)->fdir, 0);\n> +\t\t\t\t&((struct i40e_fdir_filter *)flow->rule)->fdir,\n> +\t\t\t\t0, false);\n> \n> \t\t/* If the last flow is destroyed, disable fdir. */\n> \t\tif (!ret && TAILQ_EMPTY(&pf->fdir.fdir_list)) { @@ -5428,11\n> +5498,27 @@ i40e_flow_destroy(struct rte_eth_dev *dev,\n> \n> \tif (!ret) {\n> \t\tTAILQ_REMOVE(&pf->flow_list, flow, node);\n> -\t\trte_free(flow);\n> -\t} else\n> +\t\tif (filter_type == RTE_ETH_FILTER_FDIR) {\n> +\t\t\tf = FLOW_TO_FLOW_BITMAP(flow);\n> +\t\t\trte_bitmap_set(fdir_info->fdir_flow_bitmap.b, f->idx);\n> +\t\t\tfdir_info->fdir_actual_cnt--;\n> +\n> +\t\t\tif (fdir_info->fdir_invalprio == 1)\n> +\t\t\t\t/* check if the budget will be gained back to\n> +\t\t\t\t * guaranteed space\n> +\t\t\t\t */\n> +\t\t\t\tif (fdir_info->fdir_guarantee_free_space <\n> +\t\t\t\t\tfdir_info->fdir_guarantee_available_space)\n> +\t\t\t\t\tfdir_info->fdir_guarantee_free_space++;\n> +\t\t} else {\n> +\t\t\trte_free(flow);\n> +\t\t}\n> +\n> +\t} else {\n> \t\trte_flow_error_set(error, -ret,\n> \t\t\t\t RTE_FLOW_ERROR_TYPE_HANDLE, NULL,\n> \t\t\t\t \"Failed to destroy flow.\");\n> +\t}\n> \n> \treturn ret;\n> }\n> @@ -5582,6 +5668,7 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)\n> \tstruct rte_flow *flow;\n> \tvoid *temp;\n> \tint ret;\n> +\tuint32_t i = 0;\n> \n> \tret = i40e_fdir_flush(dev);\n> \tif (!ret) {\n> @@ -5597,10 +5684,24 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)\n> \t\tTAILQ_FOREACH_SAFE(flow, &pf->flow_list, node, temp) {\n> \t\t\tif (flow->filter_type == RTE_ETH_FILTER_FDIR) {\n> \t\t\t\tTAILQ_REMOVE(&pf->flow_list, flow, node);\n> -\t\t\t\trte_free(flow);\n> \t\t\t}\n> \t\t}\n> \n> +\t\t/* clear bitmap */\n> +\t\trte_bitmap_reset(fdir_info->fdir_flow_bitmap.b);\n> +\t\tfor (i = 0; i < fdir_info->fdir_space_size; i++) {\n> +\t\t\tfdir_info->fdir_flow_bitmap.fdir_flow[i].idx = i;\n> +\t\t\trte_bitmap_set(fdir_info->fdir_flow_bitmap.b, i);\n> +\t\t}\n> +\n> +\t\tfdir_info->fdir_actual_cnt = 0;\n> +\t\tfdir_info->fdir_guarantee_free_space =\n> +\t\t\tfdir_info->fdir_guarantee_available_space;\n> +\t\tmemset(fdir_info->fdir_filter_array,\n> +\t\t\t0,\n> +\t\t\tsizeof(struct i40e_fdir_filter) *\n> +\t\t\tI40E_MAX_FDIR_FILTER_NUM);\n> +\n> \t\tfor (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;\n> \t\t pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)\n> \t\t\tpf->fdir.inset_flag[pctype] = 0;\n> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index\n> 840b6f387..f5235864d 100644\n> --- a/drivers/net/i40e/i40e_rxtx.c\n> +++ b/drivers/net/i40e/i40e_rxtx.c\n> @@ -2938,16 +2938,17 @@ i40e_dev_free_queues(struct rte_eth_dev\n> *dev)\n> \t}\n> }\n> \n> -#define I40E_FDIR_NUM_TX_DESC I40E_MIN_RING_DESC -#define\n> I40E_FDIR_NUM_RX_DESC I40E_MIN_RING_DESC\n> +#define I40E_FDIR_NUM_TX_DESC 256\n> +#define I40E_FDIR_NUM_RX_DESC 256\n> \n> enum i40e_status_code\n> i40e_fdir_setup_tx_resources(struct i40e_pf *pf) {\n> \tstruct i40e_tx_queue *txq;\n> \tconst struct rte_memzone *tz = NULL;\n> -\tuint32_t ring_size;\n> +\tuint32_t ring_size, i;\n> \tstruct rte_eth_dev *dev;\n> +\tvolatile struct i40e_tx_desc *txdp;\n> \n> \tif (!pf) {\n> \t\tPMD_DRV_LOG(ERR, \"PF is not available\"); @@ -2987,6 +2988,14\n> @@ i40e_fdir_setup_tx_resources(struct i40e_pf *pf)\n> \n> \ttxq->tx_ring_phys_addr = tz->iova;\n> \ttxq->tx_ring = (struct i40e_tx_desc *)tz->addr;\n> +\n> +\t/* Set all the DD flags to 1 */\n> +\tfor (i = 0; i < I40E_FDIR_NUM_TX_DESC; i += 2) {\n> +\t\ttxdp = &txq->tx_ring[i + 1];\n> +\t\ttxdp->cmd_type_offset_bsz |=\n> I40E_TX_DESC_DTYPE_DESC_DONE;\n> +\t\ttxdp->buffer_addr = rte_cpu_to_le_64(pf->fdir.dma_addr[i / 2]);\n> +\t}\n\nDD bits are assume to be reported by hardware, why we initialize them to 1 ?\n\n> +\n> \t/*\n> \t * don't need to allocate software ring and reset for the fdir\n> \t * program queue just set the queue has been configured.\n> diff --git a/drivers/net/i40e/rte_pmd_i40e.c\n> b/drivers/net/i40e/rte_pmd_i40e.c index e216e6783..c52a5af2e 100644\n> --- a/drivers/net/i40e/rte_pmd_i40e.c\n> +++ b/drivers/net/i40e/rte_pmd_i40e.c\n> @@ -3060,7 +3060,7 @@ int\n> rte_pmd_i40e_flow_add_del_packet_template(\n> \t\t(enum i40e_fdir_status)conf->action.report_status;\n> \tfilter_conf.action.flex_off = conf->action.flex_off;\n> \n> -\treturn i40e_flow_add_del_fdir_filter(dev, &filter_conf, add);\n> +\treturn i40e_flow_add_del_fdir_filter(dev, &filter_conf, add, true);\n> }\n> \n> int\n> --\n> 2.17.1", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from dpdk.org (dpdk.org [92.243.14.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id 73A4FA0526;\n\tFri, 10 Jul 2020 04:50:40 +0200 (CEST)", "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 7CEE81DD2F;\n\tFri, 10 Jul 2020 04:50:39 +0200 (CEST)", "from mga06.intel.com (mga06.intel.com [134.134.136.31])\n by dpdk.org (Postfix) with ESMTP id 5C89F1DD2E\n for <dev@dpdk.org>; Fri, 10 Jul 2020 04:50:36 +0200 (CEST)", "from orsmga001.jf.intel.com ([10.7.209.18])\n by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n 09 Jul 2020 19:50:35 -0700", "from fmsmsx103.amr.corp.intel.com ([10.18.124.201])\n by orsmga001.jf.intel.com with ESMTP; 09 Jul 2020 19:50:35 -0700", "from FMSMSX109.amr.corp.intel.com (10.18.116.9) by\n FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS)\n id 14.3.439.0; Thu, 9 Jul 2020 19:50:34 -0700", "from shsmsx151.ccr.corp.intel.com (10.239.6.50) by\n fmsmsx109.amr.corp.intel.com (10.18.116.9) with Microsoft SMTP Server (TLS)\n id 14.3.439.0; Thu, 9 Jul 2020 19:50:34 -0700", "from shsmsx103.ccr.corp.intel.com ([169.254.4.22]) by\n SHSMSX151.ccr.corp.intel.com ([169.254.3.49]) with mapi id 14.03.0439.000;\n Fri, 10 Jul 2020 10:50:31 +0800" ], "IronPort-SDR": [ "\n YHWikzIHoeXpDfPF7nCQ1+WqrMkRCaXupd5Asj49PXuBIM75sttXosoUuM56I33UPGuzvDd07I\n O6kqwwAaf9og==", "\n 1nrdYGrOQx9oRcVVy0xx4HJ1xngZLZibTSIdLhVVbgkDLxNpTrpNCohC18l/ATXVf4yLpCSJOj\n TwaWg4eeFkAg==" ], "X-IronPort-AV": [ "E=McAfee;i=\"6000,8403,9677\"; a=\"209671941\"", "E=Sophos;i=\"5.75,334,1589266800\"; d=\"scan'208\";a=\"209671941\"", "E=Sophos;i=\"5.75,334,1589266800\"; d=\"scan'208\";a=\"358645689\"" ], "X-Amp-Result": "SKIPPED(no attachment in message)", "X-Amp-File-Uploaded": "False", "X-ExtLoop1": "1", "From": "\"Zhang, Qi Z\" <qi.z.zhang@intel.com>", "To": "\"Sun, Chenmin\" <chenmin.sun@intel.com>, \"Xing, Beilei\"\n <beilei.xing@intel.com>, \"Wu, Jingjing\" <jingjing.wu@intel.com>, \"Wang,\n Haiyue\" <haiyue.wang@intel.com>", "CC": "\"dev@dpdk.org\" <dev@dpdk.org>", "Thread-Topic": "[PATCH V2] net/i40e: i40e FDIR update rate optimization", "Thread-Index": "AQHWVbPRQH11/Ue+OUmexMyQ6HjNHqkADk2w", "Date": "Fri, 10 Jul 2020 02:50:30 +0000", "Message-ID": "\n <039ED4275CED7440929022BC67E706115485A08F@SHSMSX103.ccr.corp.intel.com>", "References": "<20200612180015.14760-1-chenmin.sun@intel.com>\n <20200709143932.35806-1-chenmin.sun@intel.com>", "In-Reply-To": "<20200709143932.35806-1-chenmin.sun@intel.com>", "Accept-Language": "en-US", "Content-Language": "en-US", "X-MS-Has-Attach": "", "X-MS-TNEF-Correlator": "", "dlp-product": "dlpe-windows", "dlp-version": "11.2.0.6", "dlp-reaction": "no-action", "x-originating-ip": "[10.239.127.40]", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "quoted-printable", "MIME-Version": "1.0", "Subject": "Re: [dpdk-dev] [PATCH V2] net/i40e: i40e FDIR update rate\n\toptimization", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 115821, "web_url": "https://patches.dpdk.org/comment/115821/", "msgid": "<SN6PR11MB2829CDEB1124DE6CB530EA19E3600@SN6PR11MB2829.namprd11.prod.outlook.com>", "list_archive_url": "https://inbox.dpdk.org/dev/SN6PR11MB2829CDEB1124DE6CB530EA19E3600@SN6PR11MB2829.namprd11.prod.outlook.com", "date": "2020-07-13T08:33:04", "subject": "Re: [dpdk-dev] [PATCH V2] net/i40e: i40e FDIR update rate\n\toptimization", "submitter": { "id": 1212, "url": "https://patches.dpdk.org/api/people/1212/?format=api", "name": "Chenmin Sun", "email": "chenmin.sun@intel.com" }, "content": "Best Regards,\nSun, Chenmin\n\n> -----Original Message-----\n> From: Zhang, Qi Z <qi.z.zhang@intel.com>\n> Sent: Friday, July 10, 2020 10:51 AM\n> To: Sun, Chenmin <chenmin.sun@intel.com>; Xing, Beilei\n> <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Wang,\n> Haiyue <haiyue.wang@intel.com>\n> Cc: dev@dpdk.org\n> Subject: RE: [PATCH V2] net/i40e: i40e FDIR update rate optimization\n> \n> \n> \n> > -----Original Message-----\n> > From: Sun, Chenmin <chenmin.sun@intel.com>\n> > Sent: Thursday, July 9, 2020 10:40 PM\n> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei\n> > <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Wang,\n> > Haiyue <haiyue.wang@intel.com>\n> > Cc: dev@dpdk.org; Sun, Chenmin <chenmin.sun@intel.com>\n> > Subject: [PATCH V2] net/i40e: i40e FDIR update rate optimization\n> >\n> > From: Chenmin Sun <chenmin.sun@intel.com>\n> >\n> > This patch optimized the fdir update rate for i40e PF, by tracking\n> > whether the fdir rule being inserted into the guaranteed space or shared\n> space.\n> > For the flows that are inserted to the guaranteed space, we assume\n> > that the insertion will always succeed as the hardware only reports\n> > the \"no enough space left\" error. In this case, the software can\n> > directly return success and no need to retrieve the result from the\n> > hardware. See the fdir programming status descriptor format in the\n> datasheet for more details.\n> >\n> > This patch changes the global register GLQF_CTL. Therefore, when\n> > devarg ``support-multi-driver`` is set, the patch will not take effect\n> > to avoid affecting the normal behavior of other i40e drivers, e.g., Linux\n> kernel driver.\n> \n> Overall I think the patch is too big, is that possible to separate into 2 or more\n> patches?\n> \nOK\n\n\n> For example:\n> 1.) you introduce some new data structure to tack the flow\n> 2) the optimization for flow programming.\n> \n> More comments inline.\n> >\n> > Signed-off-by: Chenmin Sun <chenmin.sun@intel.com>\n> > ---\n> >\n> > v2:\n> > * Refine commit message and code comments.\n> > * Refine code style.\n> > * Fixed several memory free bugs.\n> > * Replace the bin_serch() with rte_bsf64()\n> > ---\n> > drivers/net/i40e/i40e_ethdev.c | 136 ++++++++++++++++++++++-\n> > drivers/net/i40e/i40e_ethdev.h | 63 ++++++++---\n> > drivers/net/i40e/i40e_fdir.c | 190 +++++++++++++++++++++-----------\n> > drivers/net/i40e/i40e_flow.c | 167 ++++++++++++++++++++++------\n> > drivers/net/i40e/i40e_rxtx.c | 15 ++-\n> > drivers/net/i40e/rte_pmd_i40e.c | 2 +-\n> > 6 files changed, 455 insertions(+), 118 deletions(-)\n> >\n> > diff --git a/drivers/net/i40e/i40e_ethdev.c\n> > b/drivers/net/i40e/i40e_ethdev.c index 3bc312c11..099f4c5e3 100644\n> > --- a/drivers/net/i40e/i40e_ethdev.c\n> > +++ b/drivers/net/i40e/i40e_ethdev.c\n> > @@ -26,6 +26,7 @@\n> > #include <rte_dev.h>\n> > #include <rte_tailq.h>\n> > #include <rte_hash_crc.h>\n> > +#include <rte_bitmap.h>\n> >\n> > #include \"i40e_logs.h\"\n> > #include \"base/i40e_prototype.h\"\n> > @@ -1045,8 +1046,17 @@ static int\n> > i40e_init_fdir_filter_list(struct rte_eth_dev *dev) {\n> > \tstruct i40e_pf *pf =\n> > I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);\n> > +\tstruct i40e_hw *hw = I40E_PF_TO_HW(pf);\n> > \tstruct i40e_fdir_info *fdir_info = &pf->fdir;\n> > \tchar fdir_hash_name[RTE_HASH_NAMESIZE];\n> > +\tvoid *mem = NULL;\n> > +\tuint32_t i = 0;\n> > +\tuint32_t bmp_size;\n> > +\tuint32_t alloc = 0;\n> > +\tuint32_t best = 0;\n> > +\tuint32_t pfqf_fdalloc = 0;\n> > +\tuint32_t glqf_ctl_reg = 0;\n> > +\tstruct rte_bitmap *bmp = NULL;\n> > \tint ret;\n> \n> Its better to follow RCT, and please apply on other functions.\n> \nOK\n\n\n> >\n> > \tstruct rte_hash_parameters fdir_hash_params = { @@ -1067,6\n> +1077,7\n> > @@ i40e_init_fdir_filter_list(struct rte_eth_dev *dev)\n> > \t\tPMD_INIT_LOG(ERR, \"Failed to create fdir hash table!\");\n> > \t\treturn -EINVAL;\n> > \t}\n> > +\n> > \tfdir_info->hash_map = rte_zmalloc(\"i40e_fdir_hash_map\",\n> > \t\t\t\t\t sizeof(struct i40e_fdir_filter *) *\n> > \t\t\t\t\t I40E_MAX_FDIR_FILTER_NUM,\n> > @@ -1077,8 +1088,100 @@ i40e_init_fdir_filter_list(struct rte_eth_dev\n> > *dev)\n> > \t\tret = -ENOMEM;\n> > \t\tgoto err_fdir_hash_map_alloc;\n> > \t}\n> > +\n> > +\tfdir_info->fdir_filter_array = rte_zmalloc(\"fdir_filter\",\n> > +\t\t\tsizeof(struct i40e_fdir_filter) *\n> > +\t\t\tI40E_MAX_FDIR_FILTER_NUM,\n> > +\t\t\t0);\n> > +\n> > +\tif (!fdir_info->fdir_filter_array) {\n> > +\t\tPMD_INIT_LOG(ERR,\n> > +\t\t\t \"Failed to allocate memory for fdir filter array!\");\n> > +\t\tret = -ENOMEM;\n> > +\t\tgoto err_fdir_filter_array_alloc;\n> > +\t}\n> > +\n> > +\tpfqf_fdalloc = i40e_read_rx_ctl(hw, I40E_PFQF_FDALLOC);\n> > +\talloc = ((pfqf_fdalloc & I40E_PFQF_FDALLOC_FDALLOC_MASK) >>\n> > +\t\t\tI40E_PFQF_FDALLOC_FDALLOC_SHIFT);\n> > +\tbest = ((pfqf_fdalloc & I40E_PFQF_FDALLOC_FDBEST_MASK) >>\n> > +\t\t\tI40E_PFQF_FDALLOC_FDBEST_SHIFT);\n> > +\n> > +\tglqf_ctl_reg = i40e_read_rx_ctl(hw, I40E_GLQF_CTL);\n> > +\tif (!pf->support_multi_driver) {\n> > +\t\tfdir_info->fdir_invalprio = 1;\n> > +\t\tglqf_ctl_reg |= I40E_GLQF_CTL_INVALPRIO_MASK;\n> > +\t\tPMD_DRV_LOG(INFO, \"FDIR INVALPRIO set to guaranteed\n> first\");\n> > +\t} else {\n> > +\t\tif (glqf_ctl_reg | I40E_GLQF_CTL_INVALPRIO_MASK) {\n> > +\t\t\tfdir_info->fdir_invalprio = 1;\n> > +\t\t\tPMD_DRV_LOG(INFO, \"FDIR INVALPRIO is:\n> guaranteed first\");\n> > +\t\t} else {\n> > +\t\t\tfdir_info->fdir_invalprio = 0;\n> > +\t\t\tPMD_DRV_LOG(INFO, \"FDIR INVALPRIO is: shared\n> first\");\n> > +\t\t}\n> > +\t}\n> > +\n> > +\ti40e_write_rx_ctl(hw, I40E_GLQF_CTL, glqf_ctl_reg);\n> > +\tPMD_DRV_LOG(INFO, \"FDIR guarantee space: %u, best_effort\n> > space %u.\",\n> > +\t\talloc * 32, best * 32);\n> \n> I think *32 can be applied when you assign alloc and best.\n> Also its better to replace by macro with meaningful name and use bit shift <<\n> but not *.\n\nI will get the alloc/best number from hw->func_caps instead\n\n\n> > +\n> > +\tfdir_info->fdir_space_size = (alloc + best) * 32;\n> > +\tfdir_info->fdir_actual_cnt = 0;\n> > +\tfdir_info->fdir_guarantee_available_space = alloc * 32;\n> > +\tfdir_info->fdir_guarantee_free_space =\n> > +\t\tfdir_info->fdir_guarantee_available_space;\n> > +\n> > +\tfdir_info->fdir_flow_bitmap.fdir_flow =\n> > +\t\t\trte_zmalloc(\"i40e_fdir_flows\",\n> > +\t\t\t\tsizeof(struct i40e_fdir_flows) *\n> > +\t\t\t\tfdir_info->fdir_space_size,\n> > +\t\t\t\t0);\n> > +\n> > +\tif (!fdir_info->fdir_flow_bitmap.fdir_flow) {\n> > +\t\tPMD_INIT_LOG(ERR,\n> > +\t\t\t \"Failed to allocate memory for bitmap flow!\");\n> > +\t\tret = -ENOMEM;\n> > +\t\tgoto err_fdir_bitmap_flow_alloc;\n> > +\t}\n> > +\n> > +\tfor (i = 0; i < fdir_info->fdir_space_size; i++)\n> > +\t\tfdir_info->fdir_flow_bitmap.fdir_flow[i].idx = i;\n> > +\n> > +\tbmp_size =\n> > +\t\trte_bitmap_get_memory_footprint(fdir_info-\n> >fdir_space_size);\n> > +\n> > +\tmem = rte_zmalloc(\"fdir_bmap\", bmp_size, RTE_CACHE_LINE_SIZE);\n> > +\tif (mem == NULL) {\n> > +\t\tPMD_INIT_LOG(ERR,\n> > +\t\t\t \"Failed to allocate memory for fdir bitmap!\");\n> > +\t\tret = -ENOMEM;\n> > +\t\tgoto err_fdir_mem_alloc;\n> > +\t}\n> > +\n> > +\tbmp = rte_bitmap_init(fdir_info->fdir_space_size, mem, bmp_size);\n> > +\tif (bmp == NULL) {\n> > +\t\tPMD_INIT_LOG(ERR,\n> > +\t\t\t \"Failed to initialization fdir bitmap!\");\n> > +\t\tret = -ENOMEM;\n> > +\t\tgoto err_fdir_bmp_alloc;\n> > +\t}\n> > +\n> > +\tfor (i = 0; i < fdir_info->fdir_space_size; i++)\n> > +\t\trte_bitmap_set(bmp, i);\n> > +\n> > +\tfdir_info->fdir_flow_bitmap.b = bmp;\n> > +\n> > \treturn 0;\n> >\n> > +err_fdir_bmp_alloc:\n> > +\trte_free(mem);\n> > +err_fdir_mem_alloc:\n> > +\trte_free(fdir_info->fdir_flow_bitmap.fdir_flow);\n> > +err_fdir_bitmap_flow_alloc:\n> > +\trte_free(fdir_info->fdir_filter_array);\n> > +err_fdir_filter_array_alloc:\n> > +\trte_free(fdir_info->hash_map);\n> > err_fdir_hash_map_alloc:\n> > \trte_hash_free(fdir_info->hash_table);\n> >\n> > @@ -1749,18 +1852,34 @@ i40e_rm_fdir_filter_list(struct i40e_pf *pf)\n> > \tstruct i40e_fdir_info *fdir_info;\n> >\n> > \tfdir_info = &pf->fdir;\n> > -\t/* Remove all flow director rules and hash */\n> > +\n> > +\t/* Remove all flow director rules */\n> > +\twhile ((p_fdir = TAILQ_FIRST(&fdir_info->fdir_list)))\n> > +\t\tTAILQ_REMOVE(&fdir_info->fdir_list, p_fdir, rules); }\n> > +\n> > +static void\n> > +i40e_fdir_memory_cleanup(struct i40e_pf *pf) {\n> > +\tstruct i40e_fdir_info *fdir_info;\n> > +\n> > +\tfdir_info = &pf->fdir;\n> > +\n> > +\t/* flow director memory cleanup */\n> > \tif (fdir_info->hash_map)\n> > \t\trte_free(fdir_info->hash_map);\n> > \tif (fdir_info->hash_table)\n> > \t\trte_hash_free(fdir_info->hash_table);\n> > +\tif (fdir_info->fdir_flow_bitmap.b)\n> > +\t\trte_bitmap_free(fdir_info->fdir_flow_bitmap.b);\n> > +\tif (fdir_info->fdir_flow_bitmap.fdir_flow)\n> > +\t\trte_free(fdir_info->fdir_flow_bitmap.fdir_flow);\n> > +\tif (fdir_info->fdir_filter_array)\n> > +\t\trte_free(fdir_info->fdir_filter_array);\n> >\n> > -\twhile ((p_fdir = TAILQ_FIRST(&fdir_info->fdir_list))) {\n> > -\t\tTAILQ_REMOVE(&fdir_info->fdir_list, p_fdir, rules);\n> > -\t\trte_free(p_fdir);\n> > -\t}\n> > }\n> >\n> > +\n> > void i40e_flex_payload_reg_set_default(struct i40e_hw *hw) {\n> > \t/*\n> > @@ -2618,9 +2737,14 @@ i40e_dev_close(struct rte_eth_dev *dev)\n> > \t/* Remove all flows */\n> > \twhile ((p_flow = TAILQ_FIRST(&pf->flow_list))) {\n> > \t\tTAILQ_REMOVE(&pf->flow_list, p_flow, node);\n> > -\t\trte_free(p_flow);\n> > +\t\t/* Do not free FDIR flows since they are static allocated */\n> > +\t\tif (p_flow->filter_type != RTE_ETH_FILTER_FDIR)\n> > +\t\t\trte_free(p_flow);\n> > \t}\n> >\n> > +\t/* release the fdir static allocated memory */\n> > +\ti40e_fdir_memory_cleanup(pf);\n> > +\n> > \t/* Remove all Traffic Manager configuration */\n> > \ti40e_tm_conf_uninit(dev);\n> >\n> > diff --git a/drivers/net/i40e/i40e_ethdev.h\n> > b/drivers/net/i40e/i40e_ethdev.h index 31ca05de9..5861c358b 100644\n> > --- a/drivers/net/i40e/i40e_ethdev.h\n> > +++ b/drivers/net/i40e/i40e_ethdev.h\n> > @@ -264,6 +264,15 @@ enum i40e_flxpld_layer_idx {\n> > #define I40E_DEFAULT_DCB_APP_NUM 1\n> > #define I40E_DEFAULT_DCB_APP_PRIO 3\n> >\n> > +/*\n> > + * Struct to store flow created.\n> > + */\n> > +struct rte_flow {\n> > +\tTAILQ_ENTRY(rte_flow) node;\n> > +\tenum rte_filter_type filter_type;\n> > +\tvoid *rule;\n> > +};\n> > +\n> > /**\n> > * The overhead from MTU to max frame size.\n> > * Considering QinQ packet, the VLAN tag needs to be counted twice.\n> > @@ -674,17 +683,33 @@ struct i40e_fdir_filter {\n> > \tstruct i40e_fdir_filter_conf fdir;\n> > };\n> >\n> > +struct i40e_fdir_flows {\n> \n> Why it be named as \"flows\"\n> Could you add more comment here, what's purpose of the data structure\n> and each field?\n> \nOk, I've added more comment for the new data structures, please check them in v3\n\n\n> > +\tuint32_t idx;\n> > +\tstruct rte_flow flow;\n> \n> Not sure if its better to move flow to the first field, so we cover between a\n> rte_flow point and a i40e_fdir_flow point directly.\n\nI'm not sure either... Could you explain a bit more?\nI'll move it to the first\n\n> > +};\n> > +\n> > +struct i40e_fdir_flow_bitmap {\n> > +\tstruct rte_bitmap *b;\n> > +\tstruct i40e_fdir_flows *fdir_flow;\n> > +};\n> \n> Can we just add rte_bitmap into i40e_fdir_flow?\n> \nI think the i40e_fdir_flow is a typo here, shoul be i40e_fdir_flows.\nPlease check my feedback in the next comment\n\n\n> > +\n> > +#define FLOW_TO_FLOW_BITMAP(f) \\\n> > +\tcontainer_of((f), struct i40e_fdir_flows, flow)\n> > +\n> > TAILQ_HEAD(i40e_fdir_filter_list, i40e_fdir_filter);\n> > /*\n> > * A structure used to define fields of a FDIR related info.\n> > */\n> > struct i40e_fdir_info {\n> > +#define PRG_PKT_CNT\t128\n> > +\n> > \tstruct i40e_vsi *fdir_vsi; /* pointer to fdir VSI structure */\n> > \tuint16_t match_counter_index; /* Statistic counter index used for\n> > fdir*/\n> > \tstruct i40e_tx_queue *txq;\n> > \tstruct i40e_rx_queue *rxq;\n> > -\tvoid *prg_pkt; /* memory for fdir program packet */\n> > -\tuint64_t dma_addr; /* physic address of packet\n> > memory*/\n> > +\tvoid *prg_pkt[PRG_PKT_CNT]; /* memory for fdir program packet\n> > */\n> > +\tuint64_t dma_addr[PRG_PKT_CNT];\t/* physic address of packet\n> > memory*/\n> > +\n> > \t/* input set bits for each pctype */\n> > \tuint64_t input_set[I40E_FILTER_PCTYPE_MAX];\n> > \t/*\n> > @@ -698,6 +723,27 @@ struct i40e_fdir_info {\n> > \tstruct i40e_fdir_filter **hash_map;\n> > \tstruct rte_hash *hash_table;\n> >\n> > +\tstruct i40e_fdir_filter *fdir_filter_array;\n> > +\n> > +\t/*\n> > +\t * Priority ordering at filter invalidation(destroying a flow) between\n> > +\t * \"best effort\" space and \"guaranteed\" space.\n> > +\t *\n> > +\t * 0 = At filter invalidation, the hardware first tries to increment the\n> > +\t * \"best effort\" space. The \"guaranteed\" space is incremented only\n> > when\n> > +\t * the global \"best effort\" space is at it max value or the \"best effort\"\n> > +\t * space of the PF is at its max value.\n> > +\t * 1 = At filter invalidation, the hardware first tries to increment its\n> > +\t * \"guaranteed\" space. The \"best effort\" space is incremented only\n> > when\n> > +\t * it is already at its max value.\n> > +\t */\n> > +\tuint32_t fdir_invalprio;\n> > +\tuint32_t fdir_space_size;\n> > +\tuint32_t fdir_actual_cnt;\n> > +\tuint32_t fdir_guarantee_available_space;\n> > +\tuint32_t fdir_guarantee_free_space;\n> > +\tstruct i40e_fdir_flow_bitmap fdir_flow_bitmap;\n> \n> What is the flow_bitmap usage? its free bitmap or alloc bitmap?\n> Better add more description here, or rename it with more clean purpose.\n> \nThis is neither a free bitmap nor an alloc bitmap. This bitmap manages all pre-allocated rte_flow memory pools.\nso for the above one comment, I can't move the bitmap into i40e_fdir_flows\nI have add more description in v3\n\n> > +\n> > \t/* Mark if flex pit and mask is set */\n> > \tbool flex_pit_flag[I40E_MAX_FLXPLD_LAYER];\n> > \tbool flex_mask_flag[I40E_FILTER_PCTYPE_MAX];\n> > @@ -894,15 +940,6 @@ struct i40e_mirror_rule {\n> >\n> > TAILQ_HEAD(i40e_mirror_rule_list, i40e_mirror_rule);\n> >\n> > -/*\n> > - * Struct to store flow created.\n> > - */\n> > -struct rte_flow {\n> > -\tTAILQ_ENTRY(rte_flow) node;\n> > -\tenum rte_filter_type filter_type;\n> > -\tvoid *rule;\n> > -};\n> > -\n> > TAILQ_HEAD(i40e_flow_list, rte_flow);\n> >\n> > /* Struct to store Traffic Manager shaper profile. */ @@ -1335,8\n> > +1372,8 @@ int i40e_add_del_fdir_filter(struct rte_eth_dev *dev,\n> > \t\t\t const struct rte_eth_fdir_filter *filter,\n> > \t\t\t bool add);\n> > int i40e_flow_add_del_fdir_filter(struct rte_eth_dev *dev,\n> > -\t\t\t\t const struct i40e_fdir_filter_conf *filter,\n> > -\t\t\t\t bool add);\n> > +\t\t\t const struct i40e_fdir_filter_conf *filter,\n> > +\t\t\t bool add, bool wait_status);\n> > int i40e_dev_tunnel_filter_set(struct i40e_pf *pf,\n> > \t\t\t struct rte_eth_tunnel_filter_conf *tunnel_filter,\n> > \t\t\t uint8_t add);\n> > diff --git a/drivers/net/i40e/i40e_fdir.c\n> > b/drivers/net/i40e/i40e_fdir.c index\n> > 4a778db4c..d7ba841d6 100644\n> > --- a/drivers/net/i40e/i40e_fdir.c\n> > +++ b/drivers/net/i40e/i40e_fdir.c\n> > @@ -99,7 +99,7 @@ static int\n> > i40e_flow_fdir_filter_programming(struct i40e_pf *pf,\n> > \t\t\t\t enum i40e_filter_pctype pctype,\n> > \t\t\t\t const struct i40e_fdir_filter_conf *filter,\n> > -\t\t\t\t bool add);\n> > +\t\t\t\t bool add, bool wait_status);\n> >\n> > static int\n> > i40e_fdir_rx_queue_init(struct i40e_rx_queue *rxq) @@ -163,6 +163,7\n> > @@ i40e_fdir_setup(struct i40e_pf *pf)\n> > \tchar z_name[RTE_MEMZONE_NAMESIZE];\n> > \tconst struct rte_memzone *mz = NULL;\n> > \tstruct rte_eth_dev *eth_dev = pf->adapter->eth_dev;\n> > +\tuint16_t i;\n> >\n> > \tif ((pf->flags & I40E_FLAG_FDIR) == 0) {\n> > \t\tPMD_INIT_LOG(ERR, \"HW doesn't support FDIR\"); @@ -\n> 179,6\n> > +180,7 @@ i40e_fdir_setup(struct i40e_pf *pf)\n> > \t\tPMD_DRV_LOG(INFO, \"FDIR initialization has been done.\");\n> > \t\treturn I40E_SUCCESS;\n> > \t}\n> > +\n> > \t/* make new FDIR VSI */\n> > \tvsi = i40e_vsi_setup(pf, I40E_VSI_FDIR, pf->main_vsi, 0);\n> > \tif (!vsi) {\n> > @@ -233,17 +235,27 @@ i40e_fdir_setup(struct i40e_pf *pf)\n> > \t\t\teth_dev->device->driver->name,\n> > \t\t\tI40E_FDIR_MZ_NAME,\n> > \t\t\teth_dev->data->port_id);\n> > -\tmz = i40e_memzone_reserve(z_name, I40E_FDIR_PKT_LEN,\n> > SOCKET_ID_ANY);\n> > +\tmz = i40e_memzone_reserve(z_name, I40E_FDIR_PKT_LEN *\n> > PRG_PKT_CNT,\n> > +\t\t\tSOCKET_ID_ANY);\n> > \tif (!mz) {\n> > \t\tPMD_DRV_LOG(ERR, \"Cannot init memzone for \"\n> > \t\t\t\t \"flow director program packet.\");\n> > \t\terr = I40E_ERR_NO_MEMORY;\n> > \t\tgoto fail_mem;\n> > \t}\n> > -\tpf->fdir.prg_pkt = mz->addr;\n> > -\tpf->fdir.dma_addr = mz->iova;\n> > +\n> > +\tfor (i = 0; i < PRG_PKT_CNT; i++) {\n> > +\t\tpf->fdir.prg_pkt[i] = (uint8_t *)mz->addr +\n> I40E_FDIR_PKT_LEN *\n> > +\t\t\t\t(i % PRG_PKT_CNT);\n> \n> The loop is from 0 to PRG_PKT_CNT, why \"i % PKG_PTK_CNT\"?\n> \nop \"%\" is useless, will remove it\n\n> > +\t\tpf->fdir.dma_addr[i] = mz->iova + I40E_FDIR_PKT_LEN *\n> > +\t\t\t\t(i % PRG_PKT_CNT);\n> > +\t}\n> >\n> > \tpf->fdir.match_counter_index =\n> > I40E_COUNTER_INDEX_FDIR(hw->pf_id);\n> > +\tpf->fdir.fdir_actual_cnt = 0;\n> > +\tpf->fdir.fdir_guarantee_free_space =\n> > +\t\tpf->fdir.fdir_guarantee_available_space;\n> > +\n> > \tPMD_DRV_LOG(INFO, \"FDIR setup successfully, with programming\n> queue\n> > %u.\",\n> > \t\t vsi->base_queue);\n> > \treturn I40E_SUCCESS;\n> > @@ -327,6 +339,7 @@ i40e_init_flx_pld(struct i40e_pf *pf)\n> > \t\tpf->fdir.flex_set[index].src_offset = 0;\n> > \t\tpf->fdir.flex_set[index].size =\n> I40E_FDIR_MAX_FLEXWORD_NUM;\n> > \t\tpf->fdir.flex_set[index].dst_offset = 0;\n> > +\n> Dummy empty line.\n> \nok\n\n> > \t\tI40E_WRITE_REG(hw, I40E_PRTQF_FLX_PIT(index),\n> 0x0000C900);\n> > \t\tI40E_WRITE_REG(hw,\n> > \t\t\tI40E_PRTQF_FLX_PIT(index + 1), 0x0000FC29);/*non-\n> used*/ @@\n> > -1557,11 +1570,11 @@ i40e_sw_fdir_filter_lookup(struct i40e_fdir_info\n> > *fdir_info,\n> > \treturn fdir_info->hash_map[ret];\n> > }\n> >\n> > -/* Add a flow director filter into the SW list */ static int\n> > i40e_sw_fdir_filter_insert(struct i40e_pf *pf, struct i40e_fdir_filter\n> > *filter) {\n> > \tstruct i40e_fdir_info *fdir_info = &pf->fdir;\n> > +\tstruct i40e_fdir_filter *hash_filter;\n> > \tint ret;\n> >\n> > \tif (filter->fdir.input.flow_ext.pkt_template)\n> > @@ -1577,9 +1590,14 @@ i40e_sw_fdir_filter_insert(struct i40e_pf *pf,\n> > struct i40e_fdir_filter *filter)\n> > \t\t\t ret);\n> > \t\treturn ret;\n> > \t}\n> > -\tfdir_info->hash_map[ret] = filter;\n> >\n> > -\tTAILQ_INSERT_TAIL(&fdir_info->fdir_list, filter, rules);\n> > +\tif (fdir_info->hash_map[ret])\n> > +\t\treturn -1;\n> > +\n> > +\thash_filter = &fdir_info->fdir_filter_array[ret];\n> > +\trte_memcpy(hash_filter, filter, sizeof(*filter));\n> > +\tfdir_info->hash_map[ret] = hash_filter;\n> > +\tTAILQ_INSERT_TAIL(&fdir_info->fdir_list, hash_filter, rules);\n> >\n> > \treturn 0;\n> > }\n> > @@ -1608,7 +1626,6 @@ i40e_sw_fdir_filter_del(struct i40e_pf *pf,\n> > struct i40e_fdir_input *input)\n> > \tfdir_info->hash_map[ret] = NULL;\n> >\n> > \tTAILQ_REMOVE(&fdir_info->fdir_list, filter, rules);\n> > -\trte_free(filter);\n> >\n> > \treturn 0;\n> > }\n> > @@ -1675,23 +1692,50 @@ i40e_add_del_fdir_filter(struct rte_eth_dev\n> > *dev,\n> > \treturn ret;\n> > }\n> >\n> > +static inline unsigned char *\n> > +i40e_find_available_buffer(struct rte_eth_dev *dev) {\n> > +\tstruct i40e_pf *pf =\n> > I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);\n> > +\tstruct i40e_fdir_info *fdir_info = &pf->fdir;\n> > +\tstruct i40e_tx_queue *txq = pf->fdir.txq;\n> > +\tvolatile struct i40e_tx_desc *txdp = &txq->tx_ring[txq->tx_tail + 1];\n> > +\tuint32_t i;\n> > +\n> > +\t/* wait until the tx descriptor is ready */\n> > +\tfor (i = 0; i < I40E_FDIR_MAX_WAIT_US; i++) {\n> > +\t\tif ((txdp->cmd_type_offset_bsz &\n> > +\t\t\trte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK))\n> ==\n> > +\n> \trte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))\n> > +\t\t\tbreak;\n> > +\t\trte_delay_us(1);\n> > +\t}\n> > +\tif (i >= I40E_FDIR_MAX_WAIT_US) {\n> > +\t\tPMD_DRV_LOG(ERR,\n> > +\t\t \"Failed to program FDIR filter: time out to get DD on tx\n> > queue.\");\n> > +\t\treturn NULL;\n> > +\t}\n> > +\n> > +\treturn (unsigned char *)fdir_info->prg_pkt[txq->tx_tail / 2]; }\n> \n> why tx_tail / 2, better some add some description here, and use \" >> 1\" if its\n> word / byte conversion.\n> \nBecause each fdir programming requires two tx descriptors, the number of tx descriptors is twice of fdir buffer\nwill use >> 1\n\n> > +\n> > /**\n> > * i40e_flow_add_del_fdir_filter - add or remove a flow director filter.\n> > * @pf: board private structure\n> > * @filter: fdir filter entry\n> > * @add: 0 - delete, 1 - add\n> > */\n> > +\n> > int\n> > i40e_flow_add_del_fdir_filter(struct rte_eth_dev *dev,\n> > \t\t\t const struct i40e_fdir_filter_conf *filter,\n> > -\t\t\t bool add)\n> > +\t\t\t bool add, bool wait_status)\n> > {\n> > \tstruct i40e_hw *hw =\n> > I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);\n> > \tstruct i40e_pf *pf =\n> > I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);\n> > -\tunsigned char *pkt = (unsigned char *)pf->fdir.prg_pkt;\n> > +\tunsigned char *pkt = NULL;\n> > \tenum i40e_filter_pctype pctype;\n> > \tstruct i40e_fdir_info *fdir_info = &pf->fdir;\n> > -\tstruct i40e_fdir_filter *fdir_filter, *node;\n> > +\tstruct i40e_fdir_filter *node;\n> > \tstruct i40e_fdir_filter check_filter; /* Check if the filter exists */\n> > \tint ret = 0;\n> > \treturn 0;\n> > @@ -2324,7 +2390,7 @@ i40e_fdir_filter_restore(struct i40e_pf *pf)\n> > \tuint32_t best_cnt; /**< Number of filters in best effort spaces. */\n> >\n> > \tTAILQ_FOREACH(f, fdir_list, rules)\n> > -\t\ti40e_flow_add_del_fdir_filter(dev, &f->fdir, TRUE);\n> > +\t\ti40e_flow_add_del_fdir_filter(dev, &f->fdir, TRUE, TRUE);\n> >\n> > \tfdstat = I40E_READ_REG(hw, I40E_PFQF_FDSTAT);\n> > \tguarant_cnt =\n> > diff --git a/drivers/net/i40e/i40e_flow.c\n> > b/drivers/net/i40e/i40e_flow.c index 7cd537340..260e58dc1 100644\n> > --- a/drivers/net/i40e/i40e_flow.c\n> > +++ b/drivers/net/i40e/i40e_flow.c\n> > @@ -17,6 +17,7 @@\n> > #include <rte_malloc.h>\n> > #include <rte_tailq.h>\n> > #include <rte_flow_driver.h>\n> > +#include <rte_bitmap.h>\n> >\n> > enum i40e_status_code\n> > i40e_fdir_setup_tx_resources(struct i40e_pf *pf) {\n> > \tstruct i40e_tx_queue *txq;\n> > \tconst struct rte_memzone *tz = NULL;\n> > -\tuint32_t ring_size;\n> > +\tuint32_t ring_size, i;\n> > \tstruct rte_eth_dev *dev;\n> > +\tvolatile struct i40e_tx_desc *txdp;\n> >\n> > \tif (!pf) {\n> > \t\tPMD_DRV_LOG(ERR, \"PF is not available\"); @@ -2987,6\n> +2988,14 @@\n> > i40e_fdir_setup_tx_resources(struct i40e_pf *pf)\n> >\n> > \ttxq->tx_ring_phys_addr = tz->iova;\n> > \ttxq->tx_ring = (struct i40e_tx_desc *)tz->addr;\n> > +\n> > +\t/* Set all the DD flags to 1 */\n> > +\tfor (i = 0; i < I40E_FDIR_NUM_TX_DESC; i += 2) {\n> > +\t\ttxdp = &txq->tx_ring[i + 1];\n> > +\t\ttxdp->cmd_type_offset_bsz |=\n> > I40E_TX_DESC_DTYPE_DESC_DONE;\n> > +\t\ttxdp->buffer_addr = rte_cpu_to_le_64(pf->fdir.dma_addr[i\n> / 2]);\n> > +\t}\n> \n> DD bits are assume to be reported by hardware, why we initialize them to 1 ?\n> \nYes, it may easy to confuse people here. I do this because the i40e_find_available_buffer() checks the tx descriptor.DD field to determine whether the descriptor can be used. After the NIC has taken the data of the tx descriptor, it infoms the software by setting the DD bit. The software can then safely reuse the descriptor.\nTherefore, I set DD to 1 during initialization so that the software knows that all descriptors are available.\n\n\n> >\n> > int\n> > --\n> > 2.17.1\n>", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from dpdk.org (dpdk.org [92.243.14.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id 4FB49A0540;\n\tMon, 13 Jul 2020 10:33:21 +0200 (CEST)", "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 3B14E1D52B;\n\tMon, 13 Jul 2020 10:33:20 +0200 (CEST)", "from mga17.intel.com (mga17.intel.com [192.55.52.151])\n by dpdk.org (Postfix) with ESMTP id F11401D508\n for <dev@dpdk.org>; Mon, 13 Jul 2020 10:33:17 +0200 (CEST)", "from fmsmga005.fm.intel.com ([10.253.24.32])\n by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n 13 Jul 2020 01:33:15 -0700", "from fmsmsx106.amr.corp.intel.com ([10.18.124.204])\n by fmsmga005.fm.intel.com with ESMTP; 13 Jul 2020 01:33:15 -0700", "from FMSEDG001.ED.cps.intel.com (10.1.192.133) by\n FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS)\n id 14.3.439.0; Mon, 13 Jul 2020 01:33:15 -0700", "from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.172)\n by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (TLS) id\n 14.3.439.0; Mon, 13 Jul 2020 01:33:14 -0700", "from SN6PR11MB2829.namprd11.prod.outlook.com (2603:10b6:805:62::28)\n by SN6PR11MB3053.namprd11.prod.outlook.com (2603:10b6:805:d9::16)\n with Microsoft SMTP Server (version=TLS1_2,\n cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3174.24; Mon, 13 Jul\n 2020 08:33:04 +0000", "from SN6PR11MB2829.namprd11.prod.outlook.com\n ([fe80::9db:f59b:3671:ab]) by SN6PR11MB2829.namprd11.prod.outlook.com\n ([fe80::9db:f59b:3671:ab%7]) with mapi id 15.20.3174.025; Mon, 13 Jul 2020\n 08:33:04 +0000" ], "IronPort-SDR": [ "\n sS5gaqQZHx7VA9sEb5zz4VaDOVtPHTZouMIVneixxNguJSqXvpTtWBqxgjX1oYojywa6TMVbIp\n M6RcLwHETSNg==", "\n SKAzymiPpgHreoIOvE3MxbZIfO4D3waa9esX2sr1EdTmdg7IWlN7X6piYxLDWhi/K37K0vwGSO\n acV2a5RUvZKg==" ], "X-IronPort-AV": [ "E=McAfee;i=\"6000,8403,9680\"; a=\"128631509\"", "E=Sophos;i=\"5.75,346,1589266800\"; d=\"scan'208\";a=\"128631509\"", "E=Sophos;i=\"5.75,346,1589266800\"; d=\"scan'208\";a=\"485395827\"" ], "X-Amp-Result": "SKIPPED(no attachment in message)", "X-Amp-File-Uploaded": "False", "X-ExtLoop1": "1", "ARC-Seal": "i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;\n b=Hp8j3OrU3FJ3d/EdIoaFRUheQ65mxd5QjrC4B/SN7IUNE79Y+4i1sqmQzRNPdfbLPYmZIYJzKQgKoY+jPVb8uDsTCckZs4FfhWo6UXSyCyuDAlOiSFxP1hv7zlkwhYp4sMyKDYqxPRA1u03m5FmzicA9cfm/f8jeMudz9lMuUS8ffsJFPUYx83iMpkJFyLIyan07Q2WSjdoyjIm7XBfHIYx8cCVL+4hlGP0gBfO3dFKAJdl+OjsejDKMZ2TjmcDzQn9gj4PkgctmmSG3al4psjvuadbmEbKUzUB27EBb1JTDjcri46ubdjdqIjopkT/k5MyzmmNm7EvM9jJyAUqsgA==", "ARC-Message-Signature": "i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n s=arcselector9901;\n h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n bh=RDaGIOKzoYZUgIuO8EsNmyOMP2M0r9aOc/tpTOvHaQI=;\n b=HH5/9cNnFhtuEAwWqtLTkXclaaHX+WMwB6Wel5a+m97QeluNNu5NpdlB+BUZEMGVDiiOiHv4q9nrYeQUu3cxYaVNqsjrFDDVJqogbk5VpfTH2ioI2/P0fj8ydtl8zdCMhIWv551cpfQCvgJOuJBzAiurzaVsURC3pD2FOGKZciW6wC3KNjKj+YG1E5sY3OQyi16bEew+/RoOIrNyTBf3xUEUlkSM4eqnbQpjJ735BWRfv9UpWUPAYHmcWrKKyPGNawGWYK7qM990h+47ZMU7rGxfDmF4N40R/H+DbSQzupSQpSkIFWMNr0vL8DqjYkNceA78rcT9lx+MOV6VsjCFlw==", "ARC-Authentication-Results": "i=1; mx.microsoft.com 1; spf=pass\n smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com;\n dkim=pass header.d=intel.com; arc=none", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com;\n s=selector2-intel-onmicrosoft-com;\n h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n bh=RDaGIOKzoYZUgIuO8EsNmyOMP2M0r9aOc/tpTOvHaQI=;\n b=l9EELzSwqRD9Qbg/Hg7c2qDcB3BR+XVgZCxV2ub6sm9R42E+CLtUXirzTwAMJTAJ/6quRCeg+dbDO5bI9RmgnEfB2h2fp5CVgb0iToeYr87H6ryScBmcbkbOP2UC4gdft9LKpwkHf8cHGG/RTIvYxsp12ta7gYY0juxY6Swqf+k=", "From": "\"Sun, Chenmin\" <chenmin.sun@intel.com>", "To": "\"Zhang, Qi Z\" <qi.z.zhang@intel.com>, \"Xing, Beilei\"\n <beilei.xing@intel.com>, \"Wu, Jingjing\" <jingjing.wu@intel.com>, \"Wang,\n Haiyue\" <haiyue.wang@intel.com>", "CC": "\"dev@dpdk.org\" <dev@dpdk.org>", "Thread-Topic": "[PATCH V2] net/i40e: i40e FDIR update rate optimization", "Thread-Index": "AQHWVbPUOhNXWQKnYUaXz5F6yOROQakAHlcAgAUQZnA=", "Date": "Mon, 13 Jul 2020 08:33:04 +0000", "Message-ID": "\n <SN6PR11MB2829CDEB1124DE6CB530EA19E3600@SN6PR11MB2829.namprd11.prod.outlook.com>", "References": "<20200612180015.14760-1-chenmin.sun@intel.com>\n <20200709143932.35806-1-chenmin.sun@intel.com>\n <039ED4275CED7440929022BC67E706115485A08F@SHSMSX103.ccr.corp.intel.com>", "In-Reply-To": "\n <039ED4275CED7440929022BC67E706115485A08F@SHSMSX103.ccr.corp.intel.com>", "Accept-Language": "en-US", "Content-Language": "en-US", "X-MS-Has-Attach": "", "X-MS-TNEF-Correlator": "", "authentication-results": "intel.com; dkim=none (message not signed)\n header.d=none;intel.com; dmarc=none action=none header.from=intel.com;", "x-originating-ip": "[192.102.204.45]", "x-ms-publictraffictype": "Email", "x-ms-office365-filtering-correlation-id": "727bf959-df33-40c9-9cf8-08d827075cd6", "x-ms-traffictypediagnostic": "SN6PR11MB3053:", "x-ld-processed": "46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr", "x-ms-exchange-transport-forked": "True", "x-microsoft-antispam-prvs": "\n <SN6PR11MB305319DBC238C52E07658551E3600@SN6PR11MB3053.namprd11.prod.outlook.com>", "x-ms-oob-tlc-oobclassifiers": "OLM:9508;", "x-ms-exchange-senderadcheck": "1", "x-microsoft-antispam": "BCL:0;", "x-microsoft-antispam-message-info": "\n kcmHZEMVibuN+X9iLxWHoQa64XhlpCBb2A2Mtkr3cmngCP0qwF3M279cm5pnR7fxHpEeJ83Cppa5rJ2F6q1TYK5xuRjZvQJU9tSuJiIhYl+ieih02Vase/hvZKjJ/c8zbkEyXShsz8rnNRoMVaJvySfj64yPcroHS9EKFJp1VFf6J/6s7KoHjFthUwoO4UpR9iTO0uKEIQGb6shbtVn5TsW1KaZZmXND4CXh+Y5XlQMMqgYZu3U/N67ZN6gWx7kT12rB36lTjqG7Z594kpVMw7Wuu5hHr0F4jr3ClJGYIvt5pBRwhN8/X/MCdv0NnmI4uIUJn7CoDKTrLmVzrjzfaA==", "x-forefront-antispam-report": "CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n IPV:NLI; SFV:NSPM; H:SN6PR11MB2829.namprd11.prod.outlook.com; PTR:; CAT:NONE;\n SFTY:;\n SFS:(4636009)(136003)(366004)(39860400002)(346002)(396003)(376002)(4326008)(8936002)(83380400001)(5660300002)(478600001)(26005)(186003)(110136005)(33656002)(15650500001)(9686003)(30864003)(52536014)(55016002)(7696005)(71200400001)(8676002)(2906002)(86362001)(6636002)(66946007)(64756008)(66556008)(66476007)(316002)(76116006)(6506007)(53546011)(66446008)(579004);\n DIR:OUT; SFP:1102;", "x-ms-exchange-antispam-messagedata": "\n TqtmGIL4aV20CCQQBymzxFzYlIkRNjg1n/3jleSmupE0S4/8N1fg3wzVKLVo3NnE33JD+LWQ/R3BJ8xS7bB13KPLbK9bxzj3ZxovjMSzCPOeMayRvKv6DfSXt5a7WeXIIunsXeE1ckA/IMiudDuOGoFjPzyLbN4JiO2mMxWWIQUgf30N7H8heviy0OjLdTjgy1OI7iXwzM1kR1GvknDCtDhfNAe9sPmewpA/9EgM5RFxH9NAtg1L42kf+oRiO8NGiSZFRBj/np5J1bGvmJ2OiyEmVfML1ploB9AQedwVPJjrQrkdGeyb2LU8tLbsVqDShylPa6nhEgtq5oe8gwGmafIl/VUb1ak32RMfT+ZmxaGBvRzBpXoe4uIp+MTn5YW7Ht+PjnCUt3JTLL4tOES0ru/Oz20nSHVKhhNViH14ojN19cYJBhrA08OYc8xIoFQ/sUSc1msqFWGhJIVEkxwp0uQUwcn5Qp2rjFWGinPrHmtCxJBVs+xxCHY6lgXeVImU", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "quoted-printable", "MIME-Version": "1.0", "X-MS-Exchange-CrossTenant-AuthAs": "Internal", "X-MS-Exchange-CrossTenant-AuthSource": "SN6PR11MB2829.namprd11.prod.outlook.com", "X-MS-Exchange-CrossTenant-Network-Message-Id": "\n 727bf959-df33-40c9-9cf8-08d827075cd6", "X-MS-Exchange-CrossTenant-originalarrivaltime": "13 Jul 2020 08:33:04.7688 (UTC)", "X-MS-Exchange-CrossTenant-fromentityheader": "Hosted", "X-MS-Exchange-CrossTenant-id": "46c98d88-e344-4ed4-8496-4ed7712e255d", "X-MS-Exchange-CrossTenant-mailboxtype": "HOSTED", "X-MS-Exchange-CrossTenant-userprincipalname": "\n +w4WS8zduRp1XdbCu9lgFgymKtb9Qu8ttVMjUhr4NdmK4rws8vst4Pffb8nqR5Untz4wjnkTvHBbD3ynAf5kMA==", "X-MS-Exchange-Transport-CrossTenantHeadersStamped": "SN6PR11MB3053", "X-OriginatorOrg": "intel.com", "Subject": "Re: [dpdk-dev] [PATCH V2] net/i40e: i40e FDIR update rate\n\toptimization", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null } ]