From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 1F1861B2E5 for ; Sat, 21 Oct 2017 14:08:00 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Oct 2017 05:07:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,410,1503385200"; d="scan'208";a="165343980" Received: from irsmsx154.ger.corp.intel.com ([163.33.192.96]) by fmsmga005.fm.intel.com with ESMTP; 21 Oct 2017 05:07:58 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.167]) by IRSMSX154.ger.corp.intel.com ([169.254.12.83]) with mapi id 14.03.0319.002; Sat, 21 Oct 2017 13:07:56 +0100 From: "Iremonger, Bernard" To: "Singh, Jasvinder" , "dev@dpdk.org" , "Yigit, Ferruh" , "Ananyev, Konstantin" , "Dumitrescu, Cristian" , "adrien.mazarguil@6wind.com" CC: "Iremonger, Bernard" Thread-Topic: [PATCH v8 1/4] librte_flow_classify: add flow classify library Thread-Index: AQHTR4Y4aDMwvqINj0GpJmxjcjYtI6LrK4KAgAHHyqCAAURN8A== Date: Sat, 21 Oct 2017 12:07:56 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C24E054832@IRSMSX108.ger.corp.intel.com> References: <1506936668-31197-1-git-send-email-bernard.iremonger@intel.com> <1508271970-2229-2-git-send-email-bernard.iremonger@intel.com> <54CBAA185211B4429112C315DA58FF6D33275162@IRSMSX103.ger.corp.intel.com> <8CEF83825BEC744B83065625E567D7C24E0545D6@IRSMSX108.ger.corp.intel.com> In-Reply-To: <8CEF83825BEC744B83065625E567D7C24E0545D6@IRSMSX108.ger.corp.intel.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMGYyMzY1YmItZjIxZi00MzAxLWJmNGMtNzA1YzkzYjgwYzIwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IkpGYzNVSVIrSHBVSWZqbndjR2trWEZMUzdTNDRXZThRZnBwV2p1bXdGbHM9In0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v8 1/4] librte_flow_classify: add flow classify library X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 21 Oct 2017 12:08:02 -0000 Hi Jasvinder > > > > > + > > > +struct acl_keys { > > > + struct rte_table_acl_rule_add_params key_add; /**< add key */ > > > + struct rte_table_acl_rule_delete_params key_del; /**< delete > > > key */ > > > +}; > > > + > > > +struct rte_flow_classify_rule { > > > + uint32_t id; /**< unique ID of classify rule */ > > > + enum rte_flow_classify_rule_type rule_type; /**< classify rule > > > +type > > > */ > > > + struct rte_flow_action action; /**< action when match found */ > > > + struct rte_flow_classify_ipv4_5tuple ipv4_5tuple; /**< ipv4 5tuple = */ > > > + union { > > > + struct acl_keys key; > > > + } u; > > > + int key_found; /**< rule key found in table */ > > > + void *entry; /**< pointer to buffer to hold rule meta data*/ > > > + void *entry_ptr; /**< handle to the table entry for rule meta > > > +data*/ }; > > > > In my opnion, the above struct should have the provision to > > accommodate other types of rules, not only ipv4_5tuples. > > Making this structure modular will help in extending it for other rule > > types in the future. >=20 > I will refactor by adding struct classify_rules{} to struct > rte_flow_classif_rule{}: >=20 > struct classify_rules { > enum rte_flow_classify_rule_type type; > union { > struct rte_flow_classify_ipv4_5tuple ipv4_5tuple; > } u; > }; >=20 > struct rte_flow_classify_rule { > uint32_t id; /* unique ID of classify rule */ > struct rte_flow_action action; /* action when match found */ > struct classify_rules rules; /* union of rules */ > union { > struct acl_keys key; > } u; > int key_found; /* rule key found in table */ > void *entry; /* pointer to buffer to hold rule meta data */ > void *entry_ptr; /* handle to the table entry for rule meta data */ }; >=20 >=20 > > > +int > > > +rte_flow_classify_validate( > > > + const struct rte_flow_attr *attr, > > > + const struct rte_flow_item pattern[], > > > + const struct rte_flow_action actions[], > > > + struct rte_flow_error *error) > > > +{ > > > + struct rte_flow_item *items; > > > + parse_filter_t parse_filter; > > > + uint32_t item_num =3D 0; > > > + uint32_t i =3D 0; > > > + int ret; > > > + > > > + if (!error) > > > + return -EINVAL; > > > + > > > + if (!pattern) { > > > + rte_flow_error_set(error, EINVAL, > > > RTE_FLOW_ERROR_TYPE_ITEM_NUM, > > > + NULL, "NULL pattern."); > > > + return -EINVAL; > > > + } > > > + > > > + if (!actions) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_ACTION_NUM, > > > + NULL, "NULL action."); > > > + return -EINVAL; > > > + } > > > + > > > + if (!attr) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_ATTR, > > > + NULL, "NULL attribute."); > > > + return -EINVAL; > > > + } > > > + > > > + memset(&ntuple_filter, 0, sizeof(ntuple_filter)); > > > + > > > + /* Get the non-void item number of pattern */ > > > + while ((pattern + i)->type !=3D RTE_FLOW_ITEM_TYPE_END) { > > > + if ((pattern + i)->type !=3D RTE_FLOW_ITEM_TYPE_VOID) > > > + item_num++; > > > + i++; > > > + } > > > + item_num++; > > > + > > > + items =3D malloc(item_num * sizeof(struct rte_flow_item)); > > > + if (!items) { > > > + rte_flow_error_set(error, ENOMEM, > > > RTE_FLOW_ERROR_TYPE_ITEM_NUM, > > > + NULL, "No memory for pattern items."); > > > + return -ENOMEM; > > > + } > > > + > > > + memset(items, 0, item_num * sizeof(struct rte_flow_item)); > > > + classify_pattern_skip_void_item(items, pattern); > > > + > > > + parse_filter =3D classify_find_parse_filter_func(items); > > > + if (!parse_filter) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_ITEM, > > > + pattern, "Unsupported pattern"); > > > + free(items); > > > + return -EINVAL; > > > + } > > > + > > > + ret =3D parse_filter(attr, items, actions, &ntuple_filter, error); > > > + free(items); > > > + return ret; > > > +} > > > > This function mainly parses the flow pattern, actions etc and fill the > > entries in internal ntuple_filter.It is invoked only in > > flow_entry_add(). Is there any reason to make this function available a= s > public API? >=20 > This function does not need to be a public API. > The flow_classify API's started out mirroring the flow API's but this is = no > longer the case. > Probably better to make it internal and it could be made public later if > needed. >=20 >=20 > > > +#ifdef RTE_LIBRTE_CLASSIFY_DEBUG > > > +#define uint32_t_to_char(ip, a, b, c, d) do {\ > > > + *a =3D (unsigned char)(ip >> 24 & 0xff);\ > > > + *b =3D (unsigned char)(ip >> 16 & 0xff);\ > > > + *c =3D (unsigned char)(ip >> 8 & 0xff);\ > > > + *d =3D (unsigned char)(ip & 0xff);\ > > > + } while (0) > > > + > > > +static inline void > > > +print_ipv4_key_add(struct rte_table_acl_rule_add_params *key) { > > > + unsigned char a, b, c, d; > > > + > > > + printf("ipv4_key_add: 0x%02hhx/0x%hhx ", > > > + key->field_value[PROTO_FIELD_IPV4].value.u8, > > > + key->field_value[PROTO_FIELD_IPV4].mask_range.u8); > > > + > > > + uint32_t_to_char(key->field_value[SRC_FIELD_IPV4].value.u32, > > > + &a, &b, &c, &d); > > > + printf(" %hhu.%hhu.%hhu.%hhu/0x%x ", a, b, c, d, > > > + key- > > > >field_value[SRC_FIELD_IPV4].mask_range.u32); > > > + > > > + uint32_t_to_char(key->field_value[DST_FIELD_IPV4].value.u32, > > > + &a, &b, &c, &d); > > > + printf("%hhu.%hhu.%hhu.%hhu/0x%x ", a, b, c, d, > > > + key- > > > >field_value[DST_FIELD_IPV4].mask_range.u32); > > > + > > > + printf("%hu : 0x%x %hu : 0x%x", > > > + key->field_value[SRCP_FIELD_IPV4].value.u16, > > > + key->field_value[SRCP_FIELD_IPV4].mask_range.u16, > > > + key->field_value[DSTP_FIELD_IPV4].value.u16, > > > + key->field_value[DSTP_FIELD_IPV4].mask_range.u16); > > > + > > > + printf(" priority: 0x%x\n", key->priority); } > > > > The above function is specific to printing acl table keys. How about > > making this function little generic by passing the parameters to > > distinguish the rule, table type, etc. and do the printing? > > > > Same comments for the print_ipv4_key_delete(). > > >=20 > This is debug code, could it be left as is until another table type is ad= ded? > I will rename to include acl in the function names. >=20 > > > > > +static inline void > > > +print_ipv4_key_delete(struct rte_table_acl_rule_delete_params *key) > { > > > + unsigned char a, b, c, d; > > > + > > > + printf("ipv4_key_del: 0x%02hhx/0x%hhx ", > > > + key->field_value[PROTO_FIELD_IPV4].value.u8, > > > + key->field_value[PROTO_FIELD_IPV4].mask_range.u8); > > > + > > > + uint32_t_to_char(key->field_value[SRC_FIELD_IPV4].value.u32, > > > + &a, &b, &c, &d); > > > + printf(" %hhu.%hhu.%hhu.%hhu/0x%x ", a, b, c, d, > > > + key- > > > >field_value[SRC_FIELD_IPV4].mask_range.u32); > > > + > > > + uint32_t_to_char(key->field_value[DST_FIELD_IPV4].value.u32, > > > + &a, &b, &c, &d); > > > + printf("%hhu.%hhu.%hhu.%hhu/0x%x ", a, b, c, d, > > > + key- > > > >field_value[DST_FIELD_IPV4].mask_range.u32); > > > + > > > + printf("%hu : 0x%x %hu : 0x%x\n", > > > + key->field_value[SRCP_FIELD_IPV4].value.u16, > > > + key->field_value[SRCP_FIELD_IPV4].mask_range.u16, > > > + key->field_value[DSTP_FIELD_IPV4].value.u16, > > > + key->field_value[DSTP_FIELD_IPV4].mask_range.u16); > > > +} > > > +#endif > > > + > > > +static int > > > +rte_flow_classifier_check_params(struct rte_flow_classifier_params > > > *params) > > > +{ > > > + if (params =3D=3D NULL) { > > > + RTE_LOG(ERR, CLASSIFY, > > > + "%s: Incorrect value for parameter params\n", > > > __func__); > > > + return -EINVAL; > > > + } > > > + > > > + /* name */ > > > + if (params->name =3D=3D NULL) { > > > + RTE_LOG(ERR, CLASSIFY, > > > + "%s: Incorrect value for parameter name\n", > > > __func__); > > > + return -EINVAL; > > > + } > > > + > > > + /* socket */ > > > + if ((params->socket_id < 0) || > > > + (params->socket_id >=3D RTE_MAX_NUMA_NODES)) { > > > + RTE_LOG(ERR, CLASSIFY, > > > + "%s: Incorrect value for parameter socket_id\n", > > > + __func__); > > > + return -EINVAL; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +struct rte_flow_classifier * > > > +rte_flow_classifier_create(struct rte_flow_classifier_params > > > +*params) { > > > + struct rte_flow_classifier *cls; > > > + int ret; > > > + > > > + /* Check input parameters */ > > > + ret =3D rte_flow_classifier_check_params(params); > > > + if (ret !=3D 0) { > > > + RTE_LOG(ERR, CLASSIFY, > > > + "%s: flow classifier params check failed (%d)\n", > > > + __func__, ret); > > > + return NULL; > > > + } > > > + > > > + /* Allocate memory for the flow classifier */ > > > + cls =3D rte_zmalloc_socket("FLOW_CLASSIFIER", > > > + sizeof(struct rte_flow_classifier), > > > + RTE_CACHE_LINE_SIZE, params->socket_id); > > > + > > > + if (cls =3D=3D NULL) { > > > + RTE_LOG(ERR, CLASSIFY, > > > + "%s: flow classifier memory allocation failed\n", > > > + __func__); > > > + return NULL; > > > + } > > > + > > > + /* Save input parameters */ > > > + snprintf(cls->name, RTE_FLOW_CLASSIFIER_MAX_NAME_SZ, "%s", > > > + params->name); > > > + cls->socket_id =3D params->socket_id; > > > + cls->type =3D params->type; > > > + > > > + /* Initialize flow classifier internal data structure */ > > > + cls->num_tables =3D 0; > > > + > > > + return cls; > > > +} > > > + > > > +static void > > > +rte_flow_classify_table_free(struct rte_table *table) { > > > + if (table->ops.f_free !=3D NULL) > > > + table->ops.f_free(table->h_table); > > > + > > > + rte_free(table->default_entry); > > > +} > > > > This is internal function. There is an API for creating a table for > > classifier instance but not for destroying the table. What if > > application requires destroying the specific table of the classifier > > but want to keep the classifier instance? >=20 > Yes, there should probably be an API to delete a table. > I will add an rte_flow_classify_table_delete() API. After further investigation, I will not add an rte_flow_table_delete() API. The tables are stored in an array. Deleting a table will leave an empty ent= ry in the array. The table_create API just adds tables to the array until the array is full= , it does handle empty entries in the array. Note, the ip_pipeline code do= es not have a table_delete API either. The ret_flow_classifier_free() API frees all the tables in the classifier. > > > +int > > > +rte_flow_classifier_free(struct rte_flow_classifier *cls) { > > > + uint32_t i; > > > + > > > + /* Check input parameters */ > > > + if (cls =3D=3D NULL) { > > > + RTE_LOG(ERR, CLASSIFY, > > > + "%s: rte_flow_classifier parameter is NULL\n", > > > + __func__); > > > + return -EINVAL; > > > + } > > > + > > > + /* Free tables */ > > > + for (i =3D 0; i < cls->num_tables; i++) { > > > + struct rte_table *table =3D &cls->tables[i]; > > > + > > > + rte_flow_classify_table_free(table); > > > + } > > > + > > > + /* Free flow classifier memory */ > > > + rte_free(cls); > > > + > > > + return 0; > > > +} > > > + > > > +static int > > > +rte_table_check_params(struct rte_flow_classifier *cls, > > > + struct rte_flow_classify_table_params *params, > > > + uint32_t *table_id) > > > +{ > > > + if (cls =3D=3D NULL) { > > > + RTE_LOG(ERR, CLASSIFY, > > > + "%s: flow classifier parameter is NULL\n", > > > + __func__); > > > + return -EINVAL; > > > + } > > > + if (params =3D=3D NULL) { > > > + RTE_LOG(ERR, CLASSIFY, "%s: params parameter is NULL\n", > > > + __func__); > > > + return -EINVAL; > > > + } > > > + if (table_id =3D=3D NULL) { > > > + RTE_LOG(ERR, CLASSIFY, "%s: table_id parameter is NULL\n", > > > + __func__); > > > + return -EINVAL; > > > + } > > > + > > > + /* ops */ > > > + if (params->ops =3D=3D NULL) { > > > + RTE_LOG(ERR, CLASSIFY, "%s: params->ops is NULL\n", > > > + __func__); > > > + return -EINVAL; > > > + } > > > + > > > + if (params->ops->f_create =3D=3D NULL) { > > > + RTE_LOG(ERR, CLASSIFY, > > > + "%s: f_create function pointer is NULL\n", __func__); > > > + return -EINVAL; > > > + } > > > + > > > + if (params->ops->f_lookup =3D=3D NULL) { > > > + RTE_LOG(ERR, CLASSIFY, > > > + "%s: f_lookup function pointer is NULL\n", > > > __func__); > > > + return -EINVAL; > > > + } > > > + > > > + /* De we have room for one more table? */ > > > + if (cls->num_tables =3D=3D RTE_FLOW_CLASSIFY_TABLE_MAX) { > > > + RTE_LOG(ERR, CLASSIFY, > > > + "%s: Incorrect value for num_tables parameter\n", > > > + __func__); > > > + return -EINVAL; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +int > > > +rte_flow_classify_table_create(struct rte_flow_classifier *cls, > > > + struct rte_flow_classify_table_params *params, > > > + uint32_t *table_id) > > > +{ > > > + struct rte_table *table; > > > + struct rte_flow_classify_table_entry *default_entry; > > > + void *h_table; > > > + uint32_t entry_size, id; > > > + int ret; > > > + > > > + /* Check input arguments */ > > > + ret =3D rte_table_check_params(cls, params, table_id); > > > + if (ret !=3D 0) > > > + return ret; > > > + > > > + id =3D cls->num_tables; > > > + table =3D &cls->tables[id]; > > > + > > > + /* Allocate space for the default table entry */ > > > + entry_size =3D sizeof(struct rte_flow_classify_table_entry) + > > > + params->table_metadata_size; > > > + default_entry =3D > > > + (struct rte_flow_classify_table_entry *) rte_zmalloc_socket( > > > + "Flow Classify default entry", entry_size, > > > + RTE_CACHE_LINE_SIZE, cls->socket_id); > > > + if (default_entry =3D=3D NULL) { > > > + RTE_LOG(ERR, CLASSIFY, > > > + "%s: Failed to allocate default entry\n", __func__); > > > + return -EINVAL; > > > + } > > > > what is the purpose of default_entry as I don't see its usage anywhere > > in the library? >=20 > This came from the ip_pipeline code in earlier discussions, it is not use= d at > present. > I will remove it. >=20 >=20 > > > + /* Create the table */ > > > + h_table =3D params->ops->f_create(params->arg_create, cls- > > > >socket_id, > > > + entry_size); > > > + if (h_table =3D=3D NULL) { > > > + rte_free(default_entry); > > > + RTE_LOG(ERR, CLASSIFY, "%s: Table creation failed\n", > > > __func__); > > > + return -EINVAL; > > > + } > > > + > > > + /* Commit current table to the classifier */ > > > + cls->num_tables++; > > > + *table_id =3D id; > > > + > > > + /* Save input parameters */ > > > + memcpy(&table->ops, params->ops, sizeof(struct rte_table_ops)); > > > + > > > + table->entry_size =3D entry_size; > > > + table->default_entry =3D default_entry; > > > + > > > + /* Initialize table internal data structure */ > > > + table->h_table =3D h_table; > > > + > > > + return 0; > > > +} > > > + > > > +static struct rte_flow_classify_rule * > > > +allocate_ipv4_5tuple_rule(void) > > > +{ > > > + struct rte_flow_classify_rule *rule; > > > + > > > + rule =3D malloc(sizeof(struct rte_flow_classify_rule)); > > > + if (!rule) > > > + return rule; > > > + > > > + memset(rule, 0, sizeof(struct rte_flow_classify_rule)); > > > + rule->id =3D unique_id++; > > > + rule->rule_type =3D RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_5TUPLE; > > > + > > > + memcpy(&rule->action, classify_get_flow_action(), > > > + sizeof(struct rte_flow_action)); > > > + > > > + /* key add values */ > > > + rule->u.key.key_add.priority =3D ntuple_filter.priority; > > > + rule- > > > >u.key.key_add.field_value[PROTO_FIELD_IPV4].mask_range.u8 =3D > > > + ntuple_filter.proto_mask; > > > + rule->u.key.key_add.field_value[PROTO_FIELD_IPV4].value.u8 =3D > > > + ntuple_filter.proto; > > > + rule->ipv4_5tuple.proto =3D ntuple_filter.proto; > > > + rule->ipv4_5tuple.proto_mask =3D ntuple_filter.proto_mask; > > > + > > > + rule->u.key.key_add.field_value[SRC_FIELD_IPV4].mask_range.u32 > > > =3D > > > + ntuple_filter.src_ip_mask; > > > + rule->u.key.key_add.field_value[SRC_FIELD_IPV4].value.u32 =3D > > > + ntuple_filter.src_ip; > > > + rule->ipv4_5tuple.src_ip_mask =3D ntuple_filter.src_ip_mask; > > > + rule->ipv4_5tuple.src_ip =3D ntuple_filter.src_ip; > > > + > > > + rule->u.key.key_add.field_value[DST_FIELD_IPV4].mask_range.u32 > > > =3D > > > + ntuple_filter.dst_ip_mask; > > > + rule->u.key.key_add.field_value[DST_FIELD_IPV4].value.u32 =3D > > > + ntuple_filter.dst_ip; > > > + rule->ipv4_5tuple.dst_ip_mask =3D ntuple_filter.dst_ip_mask; > > > + rule->ipv4_5tuple.dst_ip =3D ntuple_filter.dst_ip; > > > + > > > + rule- > > > >u.key.key_add.field_value[SRCP_FIELD_IPV4].mask_range.u16 =3D > > > + ntuple_filter.src_port_mask; > > > + rule->u.key.key_add.field_value[SRCP_FIELD_IPV4].value.u16 =3D > > > + ntuple_filter.src_port; > > > + rule->ipv4_5tuple.src_port_mask =3D ntuple_filter.src_port_mask; > > > + rule->ipv4_5tuple.src_port =3D ntuple_filter.src_port; > > > + > > > + rule- > > > >u.key.key_add.field_value[DSTP_FIELD_IPV4].mask_range.u16 =3D > > > + ntuple_filter.dst_port_mask; > > > + rule->u.key.key_add.field_value[DSTP_FIELD_IPV4].value.u16 =3D > > > + ntuple_filter.dst_port; > > > + rule->ipv4_5tuple.dst_port_mask =3D ntuple_filter.dst_port_mask; > > > + rule->ipv4_5tuple.dst_port =3D ntuple_filter.dst_port; > > > + > > > +#ifdef RTE_LIBRTE_CLASSIFY_DEBUG > > > + print_ipv4_key_add(&rule->u.key.key_add); > > > +#endif > > > + > > > + /* key delete values */ > > > + memcpy(&rule->u.key.key_del.field_value[PROTO_FIELD_IPV4], > > > + &rule->u.key.key_add.field_value[PROTO_FIELD_IPV4], > > > + NUM_FIELDS_IPV4 * sizeof(struct rte_acl_field)); > > > + > > > +#ifdef RTE_LIBRTE_CLASSIFY_DEBUG > > > + print_ipv4_key_delete(&rule->u.key.key_del); > > > +#endif > > > + return rule; > > > +} > > > + > > > +struct rte_flow_classify_rule * > > > +rte_flow_classify_table_entry_add(struct rte_flow_classifier *cls, > > > + uint32_t table_id, > > > + const struct rte_flow_attr *attr, > > > + const struct rte_flow_item pattern[], > > > + const struct rte_flow_action actions[], > > > + struct rte_flow_error *error) > > > +{ > > > + struct rte_flow_classify_rule *rule; > > > + struct rte_flow_classify_table_entry *table_entry; > > > + int ret; > > > + > > > + if (!error) > > > + return NULL; > > > + > > > + if (!cls) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > > + NULL, "NULL classifier."); > > > + return NULL; > > > + } > > > + > > > + if (table_id >=3D cls->num_tables) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > > + NULL, "invalid table_id."); > > > + return NULL; > > > + } > > > + > > > + if (!pattern) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_ITEM_NUM, > > > + NULL, "NULL pattern."); > > > + return NULL; > > > + } > > > + > > > + if (!actions) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_ACTION_NUM, > > > + NULL, "NULL action."); > > > + return NULL; > > > + } > > > + > > > + if (!attr) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_ATTR, > > > + NULL, "NULL attribute."); > > > + return NULL; > > > + } > > > + > > > + /* parse attr, pattern and actions */ > > > + ret =3D rte_flow_classify_validate(attr, pattern, actions, error); > > > + if (ret < 0) > > > + return NULL; > > > + > > > + switch (cls->type) { > > > + case RTE_FLOW_CLASSIFY_TABLE_TYPE_ACL: > > > + rule =3D allocate_ipv4_5tuple_rule(); > > > + if (!rule) > > > + return NULL; > > > + break; > > > + default: > > > + return NULL; > > > + } > > > + > > > + rule->entry =3D malloc(sizeof(struct rte_flow_classify_table_entry)= ); > > > + if (!rule->entry) { > > > + free(rule); > > > + rule =3D NULL; > > > + return NULL; > > > + } > > > + > > > + table_entry =3D rule->entry; > > > + table_entry->rule_id =3D rule->id; > > > + > > > + ret =3D cls->tables[table_id].ops.f_add( > > > + cls->tables[table_id].h_table, > > > + &rule->u.key.key_add, > > > + rule->entry, > > > + &rule->key_found, > > > + &rule->entry_ptr); > > > + if (ret) { > > > + free(rule->entry); > > > + free(rule); > > > + rule =3D NULL; > > > + return NULL; > > > + } > > > + return rule; > > > +} > > > > It is not clear if the pattern to be added already exists in the > > table? how this information will be propagated to the application? >=20 > The key found flag will be set if the key is already present. > I will add a key_found parameter to the API to return the key found data= . >=20 > > > +int > > > +rte_flow_classify_table_entry_delete(struct rte_flow_classifier *cls= , > > > + uint32_t table_id, > > > + struct rte_flow_classify_rule *rule, > > > + struct rte_flow_error *error) > > > +{ > > > + int ret =3D -EINVAL; > > > + > > > + if (!error) > > > + return ret; > > > + > > > + if (!cls) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > > + NULL, "NULL classifier."); > > > + return ret; > > > + } > > > + > > > + if (table_id >=3D cls->num_tables) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > > + NULL, "invalid table_id."); > > > + return ret; > > > + } > > > + > > > + if (!rule) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > > + NULL, "NULL rule."); > > > + return ret; > > > + } > > > + > > > + ret =3D cls->tables[table_id].ops.f_delete( > > > + cls->tables[table_id].h_table, > > > + &rule->u.key.key_del, > > > + &rule->key_found, > > > + &rule->entry); > > > > Please introduce check for f_delete, shouldn't be NULL. >=20 > I will add a check that f_delete is not NULL. >=20 >=20 > > > + > > > +int > > > +rte_flow_classifier_run(struct rte_flow_classifier *cls, > > > + uint32_t table_id, > > > + struct rte_mbuf **pkts, > > > + const uint16_t nb_pkts, > > > + struct rte_flow_error *error) > > > +{ > > > + int ret =3D -EINVAL; > > > + uint64_t pkts_mask; > > > + uint64_t lookup_hit_mask; > > > + > > > + if (!error) > > > + return ret; > > > + > > > + if (!cls || !pkts || nb_pkts =3D=3D 0) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > > + NULL, "invalid input"); > > > + return ret; > > > + } > > > + > > > + if (table_id >=3D cls->num_tables) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > > + NULL, "invalid table_id."); > > > + return ret; > > > + } > > > + > > > + pkts_mask =3D RTE_LEN2MASK(nb_pkts, uint64_t); > > > + ret =3D cls->tables[table_id].ops.f_lookup( > > > + cls->tables[table_id].h_table, > > > + pkts, pkts_mask, &lookup_hit_mask, > > > + (void **)cls->entries); > > > + > > > + if (!ret && lookup_hit_mask) > > > + cls->nb_pkts =3D nb_pkts; > > > + else > > > + cls->nb_pkts =3D 0; > > > + > > > + return ret; > > > +} > > > + > > > +static int > > > +action_apply(struct rte_flow_classifier *cls, > > > + struct rte_flow_classify_rule *rule, > > > + struct rte_flow_classify_stats *stats) { > > > + struct rte_flow_classify_ipv4_5tuple_stats *ntuple_stats; > > > + uint64_t count =3D 0; > > > + int i; > > > + int ret =3D -ENODATA; > > > + > > > + switch (rule->action.type) { > > > + case RTE_FLOW_ACTION_TYPE_COUNT: > > > + for (i =3D 0; i < cls->nb_pkts; i++) { > > > + if (rule->id =3D=3D cls->entries[i]->rule_id) > > > + count++; > > > + } > > > + if (count) { > > > + ret =3D 0; > > > + ntuple_stats =3D > > > + (struct rte_flow_classify_ipv4_5tuple_stats > > > *) > > > + stats->stats; > > > + ntuple_stats->counter1 =3D count; > > > + ntuple_stats->ipv4_5tuple =3D rule->ipv4_5tuple; > > > + } > > > + break; > > > + default: > > > + ret =3D -ENOTSUP; > > > + break; > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +int > > > +rte_flow_classifier_query(struct rte_flow_classifier *cls, > > > + struct rte_flow_classify_rule *rule, > > > + struct rte_flow_classify_stats *stats, > > > + struct rte_flow_error *error) > > > +{ > > > + int ret =3D -EINVAL; > > > + > > > + if (!error) > > > + return ret; > > > + > > > + if (!cls || !rule || !stats) { > > > + rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > > + NULL, "invalid input"); > > > + return ret; > > > + } > > > + > > > + ret =3D action_apply(cls, rule, stats); > > > + return ret; > > > +} > > > > The rte_flow_classify_run and rte_flow_classify_query API should be > > invoked consecutively in the application, true? >=20 > Yes, they should be invoked consecutively. > I will merge the rte_flow_classify_run API with the rte_flow_classify_= query > API and drop the rte_flow_classif_run API. >=20 >=20 > > > diff --git a/lib/librte_flow_classify/rte_flow_classify.h > > > b/lib/librte_flow_classify/rte_flow_classify.h > > > new file mode 100644 > > > index 0000000..9bd6cf4 > > > --- /dev/null > > > +++ b/lib/librte_flow_classify/rte_flow_classify.h > > > @@ -0,0 +1,321 @@ > > > +/*- > > > + * BSD LICENSE > > > + * > > > + * Copyright(c) 2017 Intel Corporation. All rights reserved. > > > + * All rights reserved. > > > + * > > > + * Redistribution and use in source and binary forms, with or with= out > > > + * modification, are permitted provided that the following conditi= ons > > > + * are met: > > > + * > > > + * * Redistributions of source code must retain the above copyri= ght > > > + * notice, this list of conditions and the following disclaime= r. > > > + * * Redistributions in binary form must reproduce the above > copyright > > > + * notice, this list of conditions and the following disclaime= r in > > > + * the documentation and/or other materials provided with the > > > + * distribution. > > > + * * Neither the name of Intel Corporation nor the names of its > > > + * contributors may be used to endorse or promote products der= ived > > > + * from this software without specific prior written permissio= n. > > > + * > > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND > > > CONTRIBUTORS > > > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, > BUT > > > NOT > > > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND > > > FITNESS FOR > > > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE > > > COPYRIGHT > > > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, > > > INCIDENTAL, > > > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, > > BUT > > > NOT > > > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; > > LOSS > > > OF USE, > > > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER > CAUSED > > > AND ON ANY > > > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR > > > TORT > > > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY > OUT > > OF > > > THE USE > > > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > > > DAMAGE. > > > + */ > > > + > > > +#ifndef _RTE_FLOW_CLASSIFY_H_ > > > +#define _RTE_FLOW_CLASSIFY_H_ > > > + > > > +/** > > > + * @file > > > + * > > > + * RTE Flow Classify Library > > > + * > > > + * This library provides flow record information with some measured > > > properties. > > > + * > > > + * Application should define the flow and measurement criteria > > > + (action) for > > > it. > > > + * > > > + * Library doesn't maintain any flow records itself, instead flow > > > + information > > > is > > > + * returned to upper layer only for given packets. > > > + * > > > + * It is application's responsibility to call > > > + rte_flow_classify_query() > > > + * for group of packets, just after receiving them or before > > > + transmitting > > > them. > > > + * Application should provide the flow type interested in, > > > + measurement to > > > apply > > > + * to that flow in rte_flow_classify_create() API, and should > > > + provide > > > + * rte_flow_classify object and storage to put results in > > > + * rte_flow_classify_query() API. > > > + * > > > + * Usage: > > > + * - application calls rte_flow_classify_create() to create a > > rte_flow_classify > > > + * object. > > > + * - application calls rte_flow_classify_query() in a polling manne= r, > > > + * preferably after rte_eth_rx_burst(). This will cause the libra= ry to > > > + * convert packet information to flow information with some > > > measurements. > > > + * - rte_flow_classify object can be destroyed when they are no > > > + more > > > needed > > > + * via rte_flow_classify_destroy() > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#ifdef __cplusplus > > > +extern "C" { > > > +#endif > > > + > > > + > > > +#define RTE_FLOW_CLASSIFY_TABLE_MAX 1 > > > + > > > +/** Opaque data type for flow classifier */ struct > > > +rte_flow_classifier; > > > + > > > +/** Opaque data type for flow classify rule */ struct > > > +rte_flow_classify_rule; > > > + > > > +enum rte_flow_classify_rule_type { > > > + RTE_FLOW_CLASSIFY_RULE_TYPE_NONE, /**< no type */ > > > + RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_5TUPLE, /**< IPv4 5tuple > > > type */ > > > +}; > > > + > > > +enum rte_flow_classify_table_type { > > > + RTE_FLOW_CLASSIFY_TABLE_TYPE_NONE, /**< no type */ > > > + RTE_FLOW_CLASSIFY_TABLE_TYPE_ACL, /**< ACL type */ }; > > > + > > > +/** Parameters for flow classifier creation */ struct > > > +rte_flow_classifier_params { > > > + /**< flow classifier name */ > > > + const char *name; > > > + > > > + /**< CPU socket ID where memory for the flow classifier and its */ > > > + /**< elements (tables) should be allocated */ > > > + int socket_id; > > > + > > > + /**< Table type */ > > > + enum rte_flow_classify_table_type type; > > > + > > > + /**< Table id */ > > > + uint32_t table_id; > > > +}; > > > + > > > +struct rte_flow_classify_table_params { > > > + /** > > + struct rte_table_ops *ops; > > > + > > > + /**< Opaque param to be passed to the table create operation */ > > > + void *arg_create; > > > + > > > + /**< Memory size to be reserved per classifier object entry for */ > > > + /**< storing meta data */ > > > + uint32_t table_metadata_size; > > > +}; > > > + > > > +struct rte_flow_classify_ipv4_5tuple { > > > + uint32_t dst_ip; /**< Destination IP address in big endian.= */ > > > + uint32_t dst_ip_mask; /**< Mask of destination IP address. */ > > > + uint32_t src_ip; /**< Source IP address in big endian. */ > > > + uint32_t src_ip_mask; /**< Mask of destination IP address. */ > > > + uint16_t dst_port; /**< Destination port in big endian. */ > > > + uint16_t dst_port_mask; /**< Mask of destination port. */ > > > + uint16_t src_port; /**< Source Port in big endian. */ > > > + uint16_t src_port_mask; /**< Mask of source port. */ > > > + uint8_t proto; /**< L4 protocol. */ > > > + uint8_t proto_mask; /**< Mask of L4 protocol. */ > > > +}; > > > + > > > +struct rte_flow_classify_table_entry { > > > + /**< meta-data for classify rule */ > > > + uint32_t rule_id; > > > + > > > + /**< Start of table entry area for user defined meta data */ > > > + __extension__ uint8_t meta_data[0]; }; > > > > The above structure is not used by any of the public API ? > > > > > + * Flow stats > > > + * > > > + * For the count action, stats can be returned by the query API. > > > + * > > > + * Storage for stats is provided by application. > > > + */ > > > +struct rte_flow_classify_stats { > > > + void *stats; > > > +}; > > > + > > > +struct rte_flow_classify_ipv4_5tuple_stats { > > > + /**< count of packets that match IPv4 5tuple pattern */ > > > + uint64_t counter1; > > > + /**< IPv4 5tuple data */ > > > + struct rte_flow_classify_ipv4_5tuple ipv4_5tuple; }; > > > + > > > +/** > > > + * Flow classifier create > > > + * > > > + * @param params > > > + * Parameters for flow classifier creation > > > + * @return > > > + * Handle to flow classifier instance on success or NULL otherwise > > > + */ > > > +struct rte_flow_classifier *rte_flow_classifier_create( > > > + struct rte_flow_classifier_params *params); > > > + > > > +/** > > > + * Flow classifier free > > > + * > > > + * @param cls > > > + * Handle to flow classifier instance > > > + * @return > > > + * 0 on success, error code otherwise > > > + */ > > > +int rte_flow_classifier_free(struct rte_flow_classifier *cls); > > > + > > > +/** > > > + * Flow classify table create > > > + * > > > + * @param cls > > > + * Handle to flow classifier instance > > > + * @param params > > > + * Parameters for flow_classify table creation > > > + * @param table_id > > > + * Table ID. Valid only within the scope of table IDs of the curre= nt > > > + * classifier. Only returned after a successful invocation. > > > + * @return > > > + * 0 on success, error code otherwise > > > + */ > > > +int rte_flow_classify_table_create(struct rte_flow_classifier *cls, > > > + struct rte_flow_classify_table_params *params, > > > + uint32_t *table_id); > > > + > > > +/** > > > + * Validate a flow classify rule. > > > + * > > > + * @param[in] attr > > > + * Flow rule attributes > > > + * @param[in] pattern > > > + * Pattern specification (list terminated by the END pattern item)= . > > > + * @param[in] actions > > > + * Associated actions (list terminated by the END pattern item). > > > + * @param[out] error > > > + * Perform verbose error reporting if not NULL. Structure > > > + * initialised in case of error only. > > > + * > > > + * @return > > > + * 0 on success, error code otherwise. > > > + */ > > > +int > > > +rte_flow_classify_validate( > > > + const struct rte_flow_attr *attr, > > > + const struct rte_flow_item pattern[], > > > + const struct rte_flow_action actions[], > > > + struct rte_flow_error *error); > > > + > > > +/** > > > + * Add a flow classify rule to the flow_classifer table. > > > + * > > > + * @param[in] cls > > > + * Flow classifier handle > > > + * @param[in] table_id > > > + * id of table > > > + * @param[in] attr > > > + * Flow rule attributes > > > + * @param[in] pattern > > > + * Pattern specification (list terminated by the END pattern item)= . > > > + * @param[in] actions > > > + * Associated actions (list terminated by the END pattern item). > > > + * @param[out] error > > > + * Perform verbose error reporting if not NULL. Structure > > > + * initialised in case of error only. > > > + * @return > > > + * A valid handle in case of success, NULL otherwise. > > > + */ > > > +struct rte_flow_classify_rule * > > > +rte_flow_classify_table_entry_add(struct rte_flow_classifier *cls, > > > + uint32_t table_id, > > > + const struct rte_flow_attr *attr, > > > + const struct rte_flow_item pattern[], > > > + const struct rte_flow_action actions[], > > > + struct rte_flow_error *error); > > > + > > > +/** > > > + * Delete a flow classify rule from the flow_classifer table. > > > + * > > > + * @param[in] cls > > > + * Flow classifier handle > > > + * @param[in] table_id > > > + * id of table > > > + * @param[in] rule > > > + * Flow classify rule > > > + * @param[out] error > > > + * Perform verbose error reporting if not NULL. Structure > > > + * initialised in case of error only. > > > + * @return > > > + * 0 on success, error code otherwise. > > > + */ > > > +int > > > +rte_flow_classify_table_entry_delete(struct rte_flow_classifier *cls= , > > > + uint32_t table_id, > > > + struct rte_flow_classify_rule *rule, > > > + struct rte_flow_error *error); > > > + > > > +/** > > > + * Run flow classifier for given packets. > > > + * > > > + * @param[in] cls > > > + * Flow classifier handle > > > + * @param[in] table_id > > > + * id of table > > > + * @param[in] pkts > > > + * Pointer to packets to process > > > + * @param[in] nb_pkts > > > + * Number of packets to process > > > + * @param[out] error > > > + * Perform verbose error reporting if not NULL. Structure > > > + * initialised in case of error only. > > > + * > > > + * @return > > > + * 0 on success, error code otherwise. > > > + */ > > > + > > > +int rte_flow_classifier_run(struct rte_flow_classifier *cls, > > > + uint32_t table_id, > > > + struct rte_mbuf **pkts, > > > + const uint16_t nb_pkts, > > > + struct rte_flow_error *error); > > > + > > > +/** > > > + * Query flow classifier for given rule. > > > + * > > > + * @param[in] cls > > > + * Flow classifier handle > > > + * @param[in] rule > > > + * Flow classify rule > > > + * @param[in] stats > > > + * Flow classify stats > > > + * @param[out] error > > > + * Perform verbose error reporting if not NULL. Structure > > > + * initialised in case of error only. > > > + * > > > + * @return > > > + * 0 on success, error code otherwise. > > > + */ > > > +int rte_flow_classifier_query(struct rte_flow_classifier *cls, > > > + struct rte_flow_classify_rule *rule, > > > + struct rte_flow_classify_stats *stats, > > > + struct rte_flow_error *error); > > > + > > > +#ifdef __cplusplus > > > +} > > > +#endif > > > + > > > +#endif /* _RTE_FLOW_CLASSIFY_H_ */ > > > > > > There are doxygen rendering issues in this document. Please cross > > check the header file with "make doc-api-html" output. >=20 > I will check the doxygen output. >=20 > I will send a v9 patch set with the above changes. >=20 Regards, =20 Bernard.