Hi Jasvinder,
> -----Original Message-----
> From: Singh, Jasvinder
> Sent: Friday, October 6, 2017 4:01 PM
> To: Iremonger, Bernard <bernard.iremon...@intel.com>; dev@dpdk.org;
> Yigit, Ferruh <ferruh.yi...@intel.com>; Ananyev, Konstantin
> <konstantin.anan...@intel.com>; Dumitrescu, Cristian
> <cristian.dumitre...@intel.com>; adrien.mazarg...@6wind.com
> Cc: Iremonger, Bernard <bernard.iremon...@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v7 1/4] librte_flow_classify: add
> librte_flow_classify library
>
> Hi Bernard,
>
> <snip>
>
> > +struct rte_flow_classify *
> > +rte_flow_classify_create(void *table_handle,
> > + uint32_t entry_size,
> > + 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 *flow_classify;
> > + int ret;
> > +
> > + if (!error)
> > + return NULL;
> > +
> > + if (!table_handle) {
> > + rte_flow_error_set(error, EINVAL,
> > RTE_FLOW_ERROR_TYPE_HANDLE,
> > + NULL, "NULL table_handle.");
> > + 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 = rte_flow_classify_validate(table_handle, attr, pattern,
> > + actions, error);
> > + if (ret < 0)
> > + return NULL;
> > +
> > + flow_classify = allocate_5tuple();
> > + if (!flow_classify)
> > + return NULL;
> > +
> > + flow_classify->entry = malloc(entry_size);
> > + if (!flow_classify->entry) {
> > + free(flow_classify);
> > + flow_classify = NULL;
> > + return NULL;
> > + }
> > + memset(flow_classify->entry, 0, entry_size);
> > + memmove(flow_classify->entry, &flow_classify->id,
> > sizeof(uint32_t));
> > +
> > + ret = rte_table_acl_ops.f_add(table_handle, &flow_classify-
> > >key_add,
> > + flow_classify->entry, &flow_classify->key_found,
> > + &flow_classify->entry_ptr);
> > + if (ret) {
> > + free(flow_classify->entry);
> > + free(flow_classify);
> > + flow_classify = NULL;
> > + return NULL;
> > + }
> > +
> > + return flow_classify;
> > +}
>
> The API in its current form creates the classifier object which will always
> use
> librte_acl based classification mechanism. This behavior imposes restriction
> on the application to always pass only ACL table related parameters for flow
> classification. In my opinion, API implementation should be agnostic to
> specific classification method and should be generic enough to allow
> application to select any of the available flow classification method (for
> e.g.
> acl, hash, LPM, etc.). Otherwise, this library will become another abstraction
> of librte_acl for flow classification.
>
> Also, library allows table entries to be added while creating the classifier
> object, not later. Is there any specific reason?
Thanks for reviewing this patchset.
I will rework the code so that the API's are table agnostic.
In the v7 patchset the application creates the table ACL and then classify
rules can be added and deleted at will.
I will send a v8 patchset.
Regards,
Bernard.
