From: "Xing, Beilei" <beilei.xing@intel.com>
To: "Qiao, Wenjing" <wenjing.qiao@intel.com>,
"Zhang, Yuying" <yuying.zhang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Liu, Mingxia" <mingxia.liu@intel.com>
Subject: RE: [PATCH v2 2/4] net/cpfl: add flow json parser
Date: Thu, 24 Aug 2023 07:25:09 +0000 [thread overview]
Message-ID: <LV2PR11MB5997AA43C96C72D533EC0E37F71DA@LV2PR11MB5997.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20230811100012.2078135-3-wenjing.qiao@intel.com>
> -----Original Message-----
> From: Qiao, Wenjing <wenjing.qiao@intel.com>
> Sent: Friday, August 11, 2023 6:00 PM
> To: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Liu, Mingxia <mingxia.liu@intel.com>; Qiao, Wenjing
> <wenjing.qiao@intel.com>
> Subject: [PATCH v2 2/4] net/cpfl: add flow json parser
>
> A JSON file will be used to direct DPDK CPF PMD to
> parse rte_flow tokens into low level hardware resources
> defined in a DDP package file.
>
> Signed-off-by: Wenjing Qiao <wenjing.qiao@intel.com>
> ---
> Depends-on: series-29139 ("net/cpfl: support port representor")
> ---
> drivers/net/cpfl/cpfl_flow_parser.c | 1758 +++++++++++++++++++++++++++
> drivers/net/cpfl/cpfl_flow_parser.h | 205 ++++
> drivers/net/cpfl/meson.build | 3 +
> 3 files changed, 1966 insertions(+)
> create mode 100644 drivers/net/cpfl/cpfl_flow_parser.c
> create mode 100644 drivers/net/cpfl/cpfl_flow_parser.h
>
> diff --git a/drivers/net/cpfl/cpfl_flow_parser.c
> b/drivers/net/cpfl/cpfl_flow_parser.c
> new file mode 100644
> index 0000000000..b4635813ff
> --- /dev/null
> +++ b/drivers/net/cpfl/cpfl_flow_parser.c
> @@ -0,0 +1,1758 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2023 Intel Corporation
> + */
> +
> +#include <arpa/inet.h>
> +#include <asm-generic/errno-base.h>
> +#include <stdint.h>
> +
> +#include "cpfl_flow_parser.h"
> +#include "cpfl_ethdev.h"
> +#include "rte_malloc.h"
> +
> +static enum rte_flow_item_type
> +cpfl_get_item_type_by_str(const char *type)
> +{
> + if (strcmp(type, "eth") == 0)
> + return RTE_FLOW_ITEM_TYPE_ETH;
> + else if (strcmp(type, "ipv4") == 0)
> + return RTE_FLOW_ITEM_TYPE_IPV4;
> + else if (strcmp(type, "tcp") == 0)
> + return RTE_FLOW_ITEM_TYPE_TCP;
> + else if (strcmp(type, "udp") == 0)
> + return RTE_FLOW_ITEM_TYPE_UDP;
> + else if (strcmp(type, "vxlan") == 0)
> + return RTE_FLOW_ITEM_TYPE_VXLAN;
> + else if (strcmp(type, "icmp") == 0)
> + return RTE_FLOW_ITEM_TYPE_ICMP;
> + else if (strcmp(type, "vlan") == 0)
> + return RTE_FLOW_ITEM_TYPE_VLAN;
> +
> + PMD_DRV_LOG(ERR, "Not support this type: %s.", type);
> + return RTE_FLOW_ITEM_TYPE_VOID;
> +}
> +
> +static enum rte_flow_action_type
> +cpfl_get_action_type_by_str(const char *type)
> +{
> + if (strcmp(type, "vxlan_encap") == 0)
> + return RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP;
> +
> + PMD_DRV_LOG(ERR, "Not support this type: %s.", type);
Why the function only supports vxlan_encap? It's a bit confused.
If only for vxlan_encap, better to change the function name.
> + return RTE_FLOW_ACTION_TYPE_VOID;
> +}
> +
> +static const char *
> +cpfl_json_object_to_string(json_object *object, const char *name)
> +{
> + json_object *subobject;
> +
> + if (!object) {
> + PMD_DRV_LOG(ERR, "object doesn't exist.");
> + return NULL;
> + }
> + subobject = json_object_object_get(object, name);
> + if (!subobject) {
> + PMD_DRV_LOG(ERR, "%s doesn't exist.", name);
> + return 0;
Return NULL?
> + }
> + return json_object_get_string(subobject);
> +}
> +
<...>
> +static int
> +cpfl_flow_js_pattern_key_proto_field(json_object *cjson_field,
> + struct cpfl_flow_js_pr_key_proto *js_field)
> +{
> + if (cjson_field) {
How about
if (cjson_field ! =0 )
return 0;
first?
> + int len, i;
> +
> + len = json_object_array_length(cjson_field);
> + js_field->fields_size = len;
> + if (len == 0)
> + return 0;
> + js_field->fields =
> + rte_malloc(NULL, sizeof(struct
> cpfl_flow_js_pr_key_proto_field) * len, 0);
> + if (!js_field->fields) {
> + PMD_DRV_LOG(ERR, "Failed to alloc memory.");
> + return -ENOMEM;
> + }
> + for (i = 0; i < len; i++) {
> + json_object *object;
> + const char *name, *mask;
> +
> + object = json_object_array_get_idx(cjson_field, i);
> + name = cpfl_json_object_to_string(object, "name");
> + if (!name) {
> + rte_free(js_field->fields);
> + PMD_DRV_LOG(ERR, "Can not parse string
> 'name'.");
> + return -EINVAL;
> + }
> + if (strlen(name) > CPFL_FLOW_JSON_STR_SIZE_MAX) {
> + rte_free(js_field->fields);
> + PMD_DRV_LOG(ERR, "The 'name' is too
> long.");
> + return -EINVAL;
> + }
> + memcpy(js_field->fields[i].name, name, strlen(name));
> +
> + if (js_field->type == RTE_FLOW_ITEM_TYPE_ETH ||
> + js_field->type == RTE_FLOW_ITEM_TYPE_IPV4) {
> + mask = cpfl_json_object_to_string(object,
> "mask");
> + if (!mask) {
> + rte_free(js_field->fields);
> + PMD_DRV_LOG(ERR, "Can not parse
> string 'mask'.");
> + return -EINVAL;
> + }
> + memcpy(js_field->fields[i].mask, mask,
> strlen(mask));
> + } else {
> + uint32_t mask_32b;
> + int ret;
> +
> + ret = cpfl_json_object_to_uint32(object,
> "mask", &mask_32b);
> + if (ret < 0) {
> + rte_free(js_field->fields);
> + PMD_DRV_LOG(ERR, "Can not parse
> uint32 'mask'.");
> + return -EINVAL;
> + }
> + js_field->fields[i].mask_32b = mask_32b;
> + }
> + }
> + }
> + return 0;
> +}
> +
<...>
> +
> +static int
> +cpfl_flow_js_pattern_act_fv_proto(json_object *cjson_value, struct
> cpfl_flow_js_fv *js_fv)
> +{
> + uint16_t layer = 0, offset = 0, mask = 0;
> + const char *header;
> + enum rte_flow_item_type type;
> + int ret;
> +
> + ret = cpfl_json_object_to_uint16(cjson_value, "layer", &layer);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "Can not parse 'value'.");
> + return -EINVAL;
> + }
> +
> + header = cpfl_json_object_to_string(cjson_value, "header");
> + if (!header) {
> + PMD_DRV_LOG(ERR, "Can not parse string 'header'.");
> + return -EINVAL;
> + }
> + ret = cpfl_json_object_to_uint16(cjson_value, "offset", &offset);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "Can not parse 'offset'.");
> + return -EINVAL;
> + }
> + ret = cpfl_json_object_to_uint16(cjson_value, "mask", &mask);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "Can not parse 'mask'.");
> + return -EINVAL;
> + }
> + js_fv->proto.layer = layer;
> + js_fv->proto.offset = offset;
> + js_fv->proto.mask = mask;
> + type = cpfl_get_item_type_by_str(header);
> + if (type == RTE_FLOW_ITEM_TYPE_VOID)
> + return -EINVAL;
> +
No need the blank line.
> + else
> + js_fv->proto.header = type;
> + return 0;
> +}
> +
<...>
> +static int
> +cpfl_flow_js_mr_key(json_object *cjson_mr_key, struct cpfl_flow_js_mr_key
> *js_mr_key)
> +{
> + int len, i;
> +
> + len = json_object_array_length(cjson_mr_key);
> + js_mr_key->actions = rte_malloc(NULL, sizeof(struct
> cpfl_flow_js_mr_key_action) * len, 0);
> + if (!js_mr_key->actions) {
> + PMD_DRV_LOG(ERR, "Failed to alloc memory.");
> + return -ENOMEM;
> + }
> + js_mr_key->actions_size = len;
> + for (i = 0; i < len; i++) {
> + json_object *object, *cjson_mr_key_data;
> + const char *type;
> + enum rte_flow_action_type act_type;
> +
> + object = json_object_array_get_idx(cjson_mr_key, i);
> + /* mr->key->actions->type */
> + type = cpfl_json_object_to_string(object, "type");
> + if (!type) {
> + rte_free(js_mr_key->actions);
> + PMD_DRV_LOG(ERR, "Can not parse string 'type'.");
> + return -EINVAL;
> + }
> + act_type = cpfl_get_action_type_by_str(type);
> + if (act_type == RTE_FLOW_ACTION_TYPE_VOID) {
> + rte_free(js_mr_key->actions);
> + return -EINVAL;
> + }
> + js_mr_key->actions[i].type = act_type;
> + /* mr->key->actions->data */
> + cjson_mr_key_data = json_object_object_get(object, "data");
> + if (js_mr_key->actions[i].type ==
> RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP) {
> + json_object *cjson_mr_key_proto;
> + int proto_size, j;
> + struct cpfl_flow_js_mr_key_action_vxlan_encap
> *encap;
> +
> + cjson_mr_key_proto =
> json_object_object_get(cjson_mr_key_data, "protocols");
> + encap = &js_mr_key->actions[i].encap;
> + if (!cjson_mr_key_proto) {
> + encap->proto_size = 0;
> + continue;
> + }
> + proto_size =
> json_object_array_length(cjson_mr_key_proto);
> + encap->proto_size = proto_size;
> + for (j = 0; j < proto_size; j++) {
> + const char *s;
> + json_object *subobject;
> + enum rte_flow_item_type proto_type;
> +
> + subobject =
> json_object_array_get_idx(cjson_mr_key_proto, j);
> + s = json_object_get_string(subobject);
> + proto_type = cpfl_get_item_type_by_str(s);
> + if (proto_type ==
> RTE_FLOW_ITEM_TYPE_VOID) {
> + rte_free(js_mr_key->actions);
> + PMD_DRV_LOG(ERR, "parse
> VXLAN_ENCAP failed.");
> + return -EINVAL;
> + }
> + encap->protocols[j] = proto_type;
> + }
> +
No need the blank line, please check all patches.
> + } else {
> + PMD_DRV_LOG(ERR, "not support this type: %d.",
> js_mr_key->actions[i].type);
> + return -EINVAL;
> + }
> + }
> + return 0;
> +}
> +
<...>
> +static int
> +cpfl_flow_js_mr_action(json_object *cjson_mr_act, struct
> cpfl_flow_js_mr_action *js_mr_act)
> +{
> + json_object *cjson_mr_action_data;
> + const char *type;
> +
> + /* mr->action->type */
> + type = cpfl_json_object_to_string(cjson_mr_act, "type");
> + if (!type) {
> + PMD_DRV_LOG(ERR, "Can not parse string 'type'.");
> + return -EINVAL;
> + }
> +
> + /* mr->action->data */
> + cjson_mr_action_data = json_object_object_get(cjson_mr_act, "data");
> + if (strcmp(type, "mod") == 0) {
> + json_object *layout;
> + uint16_t profile = 0;
> + int ret;
> +
> + js_mr_act->type = CPFL_JS_MR_ACTION_TYPE_MOD;
> + ret = cpfl_json_object_to_uint16(cjson_mr_action_data,
> "profile", &profile);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "Can not parse 'profile'.");
> + return -EINVAL;
> + }
> + js_mr_act->mod.prof = profile;
> + layout = json_object_object_get(cjson_mr_action_data,
> "layout");
> + ret = cpfl_flow_js_mr_layout(layout, &js_mr_act->mod);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "Can not parse layout.");
> + return ret;
> + }
> + } else {
There're two spaces after else.
> + PMD_DRV_LOG(ERR, "not support this type: %s.", type);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +cpfl_flow_js_mod_rule(json_object *json_root, struct cpfl_flow_js_parser
> *parser)
> +{
> + json_object *cjson_mr;
> + int i, len;
> +
> + cjson_mr = json_object_object_get(json_root, "modifications");
> + if (!cjson_mr) {
> + PMD_DRV_LOG(INFO, "The modifications is optional.");
> + return 0;
> + }
> +
> + len = json_object_array_length(cjson_mr);
> + parser->mr_size = len;
> + if (len == 0)
> + return 0;
Move the check before 'parser->mr_size = len;'.
> + parser->modifications = rte_malloc(NULL, sizeof(struct
> cpfl_flow_js_mr) * len, 0);
> + if (!parser->modifications) {
> + PMD_DRV_LOG(ERR, "Failed to alloc memory.");
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < len; i++) {
> + int ret;
> + json_object *object, *cjson_mr_key, *cjson_mr_action,
> *cjson_mr_key_action;
> +
> + object = json_object_array_get_idx(cjson_mr, i);
> + /* mr->key */
> + cjson_mr_key = json_object_object_get(object, "key");
> + /* mr->key->actions */
> + cjson_mr_key_action = json_object_object_get(cjson_mr_key,
> "actions");
> +
> + ret = cpfl_flow_js_mr_key(cjson_mr_key_action, &parser-
> >modifications[i].key);
> + if (ret < 0) {
> + rte_free(parser->modifications);
> + PMD_DRV_LOG(ERR, "parse mr_key failed.");
> + return -EINVAL;
> + }
> + /* mr->action */
> + cjson_mr_action = json_object_object_get(object, "action");
> + ret = cpfl_flow_js_mr_action(cjson_mr_action, &parser-
> >modifications[i].action);
> + if (ret < 0) {
> + rte_free(parser->modifications);
> + PMD_DRV_LOG(ERR, "parse mr_action failed.");
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int
> +cpfl_parser_init(json_object *json_root, struct cpfl_flow_js_parser *parser)
> +{
> + int ret = 0;
> +
> + ret = cpfl_flow_js_pattern_rule(json_root, parser);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "parse pattern_rule failed.");
> + return ret;
> + }
> + ret = cpfl_flow_js_mod_rule(json_root, parser);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "parse mod_rule failed.");
> + return ret;
This ' return ret;' can be omitted. Since it will be executed at last anyway.
> + }
> +
> + return ret;
> +}
> +
> +int
> +cpfl_parser_create(struct cpfl_flow_js_parser **flow_parser, const char
> *filename)
> +{
> + struct cpfl_flow_js_parser *parser;
> + json_object *root;
> + int ret;
> +
> + parser = rte_zmalloc("flow_parser", sizeof(struct cpfl_flow_js_parser),
> 0);
> + if (!parser) {
> + PMD_DRV_LOG(ERR, "Not enough memory to create flow
> parser.");
> + return -ENOMEM;
> + }
> + root = json_object_from_file(filename);
> + if (!root) {
> + PMD_DRV_LOG(ERR, "Can not load JSON file: %s.", filename);
> + rte_free(parser);
> + return -EINVAL;
> + }
> + ret = cpfl_parser_init(root, parser);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "parser init failed.");
> + rte_free(parser);
> + return -EINVAL;
> + }
> + *flow_parser = parser;
> +
> + ret = json_object_put(root);
> + if (ret != 1) {
> + PMD_DRV_LOG(ERR, "Free json_object failed.");
Need to free parser here.
For all the error handling, better to use goto.
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static void
> +cpfl_parser_free_pr_action(struct cpfl_flow_js_pr_action *pr_act)
> +{
> + if (pr_act->type == CPFL_JS_PR_ACTION_TYPE_SEM) {
> + if (pr_act->sem.fv)
Rte_free will check the pointer, so if condition can be omitted.
Please check all rte_free(xxx) in the patches.
> + rte_free(pr_act->sem.fv);
> + }
> +}
> +
> +int
> +cpfl_parser_destroy(struct cpfl_flow_js_parser *parser)
> +{
> + int i, j;
> +
Better to check if parser is valid.
> + for (i = 0; i < parser->pr_size; i++) {
> + struct cpfl_flow_js_pr *pattern = &parser->patterns[i];
> +
> + for (j = 0; j < pattern->key.proto_size; j++) {
> + if (pattern->key.protocols[j].fields)
> + rte_free(pattern->key.protocols[j].fields);
> + }
> + if (pattern->key.protocols)
> + rte_free(pattern->key.protocols);
> +
> + if (pattern->key.attributes)
> + rte_free(pattern->key.attributes);
> +
> + for (j = 0; j < pattern->actions_size; j++) {
> + struct cpfl_flow_js_pr_action *pr_act;
> +
> + pr_act = &pattern->actions[j];
> + cpfl_parser_free_pr_action(pr_act);
> + }
> +
> + if (pattern->actions)
> + rte_free(pattern->actions);
> + }
> + if (parser->patterns)
> + rte_free(parser->patterns);
> +
> + for (i = 0; i < parser->mr_size; i++) {
> + struct cpfl_flow_js_mr *mr = &parser->modifications[i];
> +
> + if (mr->key.actions)
> + rte_free(mr->key.actions);
> + if (mr->action.type == CPFL_JS_MR_ACTION_TYPE_MOD &&
> mr->action.mod.layout)
> + rte_free(mr->action.mod.layout);
> + }
> + if (parser->modifications)
> + rte_free(parser->modifications);
> +
> + rte_free(parser);
> + return 0;
> +}
> +
<...>
> +
> +static int
> +cpfl_parse_pr_actions(struct cpfl_flow_js_pr_action *actions,
> + int size,
> + const struct rte_flow_item *items,
> + const struct rte_flow_attr *attr,
> + struct cpfl_flow_pr_action *pr_action)
> +{
> + int i, ret;
> +
> + for (i = 0; i < size; i++) {
> + struct cpfl_flow_js_pr_action *pr_act;
> + enum cpfl_flow_pr_action_type type;
> +
> + pr_act = &actions[i];
> + /* pr->actions->type */
> + type = pr_act->type;
> + /* pr->actions->data */
> + if (attr->group % 10 == 1 && type ==
> CPFL_JS_PR_ACTION_TYPE_SEM) {
> + struct cpfl_flow_js_pr_action_sem *sem = &pr_act-
> >sem;
> +
> + pr_action->type = CPFL_JS_PR_ACTION_TYPE_SEM;
> + pr_action->sem.prof = sem->prof;
> + pr_action->sem.subprof = sem->subprof;
> + pr_action->sem.keysize = sem->keysize;
> + memset(pr_action->sem.cpfl_flow_pr_fv, 0,
> + sizeof(pr_action->sem.cpfl_flow_pr_fv));
> + ret = cpfl_parse_fieldvectors(sem->fv, sem->fv_size,
> + pr_action-
> >sem.cpfl_flow_pr_fv, items);
> + return ret;
> + } else if (attr->group > 4 || attr->group == 0) {
What does 4 mean here? How about define a macro to describe it?
> + return -EPERM;
> + }
> + }
> + return 0;
> +}
> +
> +static int
> +str2MAC(const char *mask, uint8_t *addr_bytes)
Please keep cpfl pmd function name style.
> +{
> + int i, size, j;
> + uint8_t n;
> +
> + size = strlen(mask);
> + n = 0;
> + j = 0;
> + for (i = 0; i < size; i++) {
> + char ch = mask[i];
> +
> + if (ch == ':') {
> + if (j >= RTE_ETHER_ADDR_LEN)
> + return -EINVAL;
> + addr_bytes[j++] = n;
> + n = 0;
> + } else if (ch >= 'a' && ch <= 'f') {
> + n = n * 16 + ch - 'a' + 10;
> + } else if (ch >= 'A' && ch <= 'F') {
> + n = n * 16 + ch - 'A' + 10;
> + } else if (ch >= '0' && ch <= '9') {
> + n = n * 16 + ch - '0';
> + } else {
> + return -EINVAL;
> + }
> + }
> + if (j < RTE_ETHER_ADDR_LEN)
> + addr_bytes[j++] = n;
> +
> + if (j != RTE_ETHER_ADDR_LEN)
> + return -EINVAL;
> + return 0;
> +}
> +
> +static int
> +cpfl_check_eth_mask(const char *mask, const uint8_t
> addr_bytes[RTE_ETHER_ADDR_LEN])
> +{
> + int i, ret;
> + uint8_t mask_bytes[RTE_ETHER_ADDR_LEN] = {0};
> +
> + ret = str2MAC(mask, mask_bytes);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "string to mac address failed.");
> + return -EINVAL;
> + }
> + for (i = 0; i < RTE_ETHER_ADDR_LEN; i++) {
> + if (mask_bytes[i] != addr_bytes[i])
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int
> +cpfl_check_ipv4_mask(const char *mask, rte_be32_t addr)
> +{
> + uint32_t out_addr;
> +
> + /* success return 0; invalid return -EINVAL; fail return -ENOTSUP */
> + int ret = inet_pton(AF_INET, mask, &out_addr);
> +
> + if (ret < 0)
> + return -EINVAL;
> +
> + if (out_addr != addr)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int
> +cpfl_check_eth(struct cpfl_flow_js_pr_key_proto *proto, const struct
> rte_flow_item_eth *eth_mask)
> +{
> + int field_size, j;
> + int flag_dst_addr, flag_src_addr, flag_ether_type;
> + struct cpfl_flow_js_pr_key_proto_field *field;
> +
> + if (!proto)
> + return 0;
> + /* eth_mask->dst.addr_bytes */
Could you detail the comments? Seems it's not related with the following code.
> +
> + field_size = proto->fields_size;
> + if (field_size != 0 && !eth_mask)
> + return -EINVAL;
> +
> + if (field_size == 0 && eth_mask)
> + return -EINVAL;
> +
> + if (field_size == 0 && !eth_mask)
> + return 0;
> +
> + flag_dst_addr = false;
> + flag_src_addr = false;
> + flag_ether_type = false;
> + for (j = 0; j < field_size; j++) {
> + const char *name, *s_mask;
> +
> + field = &proto->fields[j];
> + /* match: rte_flow_item_eth.dst, more see Field Mapping
> + */
> + name = field->name;
> + /* match: rte_flow_item->mask */
> + if (strcmp(name, "src_addr") == 0) {
> + s_mask = field->mask;
> + if (cpfl_check_eth_mask(s_mask, eth_mask-
> >src.addr_bytes) < 0)
> + return -EINVAL;
> + flag_src_addr = true;
> + } else if (strcmp(name, "dst_addr") == 0) {
> + s_mask = field->mask;
> + if (cpfl_check_eth_mask(s_mask, eth_mask-
> >dst.addr_bytes) < 0)
> + return -EINVAL;
> + flag_dst_addr = true;
> + } else if (strcmp(name, "ether_type") == 0) {
> + uint16_t mask = (uint16_t)field->mask_32b;
> +
> + if (mask != eth_mask->type)
> + return -EINVAL;
> + flag_ether_type = true;
> + } else {
> + /* TODO: more type... */
> + PMD_DRV_LOG(ERR, "not support this name.");
> + return -EINVAL;
> + }
> + }
> + if (!flag_src_addr) {
> + if (strcmp((const char *)eth_mask->src.addr_bytes,
> "\x00\x00\x00\x00\x00\x00") != 0)
> + return -EINVAL;
> + }
> + if (!flag_dst_addr) {
> + if (strcmp((const char *)eth_mask->dst.addr_bytes,
> "\x00\x00\x00\x00\x00\x00") != 0)
> + return -EINVAL;
> + }
> + if (!flag_ether_type) {
> + if (eth_mask->hdr.ether_type != (rte_be16_t)0)
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +cpfl_check_ipv4(struct cpfl_flow_js_pr_key_proto *proto, const struct
> rte_flow_item_ipv4 *ipv4_mask)
> +{
> + if (proto) {
How about
if (proto == NULL)
return 0;
first?
Please check all other functions.
> + int field_size, j;
> + int flag_next_proto_id, flag_src_addr, flag_dst_addr;
> + struct cpfl_flow_js_pr_key_proto_field *field;
> +
> + field_size = proto->fields_size;
> + if (field_size != 0 && !ipv4_mask)
> + return -EINVAL;
> +
> + if (field_size == 0 && ipv4_mask)
> + return -EINVAL;
> +
> + if (field_size == 0 && !ipv4_mask)
> + return 0;
> +
> + flag_dst_addr = false;
> + flag_src_addr = false;
> + flag_next_proto_id = false;
> + for (j = 0; j < field_size; j++) {
> + const char *name;
> +
> + field = &proto->fields[j];
> + name = field->name;
> + if (strcmp(name, "src_addr") == 0) {
> + /* match: rte_flow_item->mask */
> + const char *mask;
> +
> + mask = field->mask;
> + if (cpfl_check_ipv4_mask(mask, ipv4_mask-
> >hdr.src_addr) < 0)
> + return -EINVAL;
> + flag_src_addr = true;
> + } else if (strcmp(name, "dst_addr") == 0) {
> + const char *mask;
> +
> + mask = field->mask;
> + if (cpfl_check_ipv4_mask(mask, ipv4_mask-
> >hdr.dst_addr) < 0)
> + return -EINVAL;
> + flag_dst_addr = true;
> + } else if (strcmp(name, "next_proto_id") == 0) {
> + uint8_t mask;
> +
> + mask = (uint8_t)field->mask_32b;
> + if (mask != ipv4_mask->hdr.next_proto_id)
> + return -EINVAL;
> + flag_next_proto_id = true;
> + } else {
> + PMD_DRV_LOG(ERR, "not support this
> name.");
> + return -EINVAL;
> + }
> + }
> + if (!flag_src_addr) {
> + if (ipv4_mask->hdr.src_addr != (rte_be32_t)0)
> + return -EINVAL;
> + }
> + if (!flag_dst_addr) {
> + if (ipv4_mask->hdr.dst_addr != (rte_be32_t)0)
> + return -EINVAL;
> + }
> + if (!flag_next_proto_id) {
> + if (ipv4_mask->hdr.next_proto_id != (uint8_t)0)
> + return -EINVAL;
> + }
> + }
> + return 0;
> +}
> +
<...>
> +static int
> +cpfl_check_pattern_key_proto(struct cpfl_flow_js_pr_key_proto *protocols,
> + int proto_size,
> + const struct rte_flow_item *items)
> +{
> + int i, length, j = 0;
According to the coding style, split j = 0 from definition of i and length.
> +
> + length = cpfl_get_items_length(items);
> +
> + if (proto_size > length - 1)
> + return -EINVAL;
> +
> + for (i = 0; i < proto_size; i++) {
> + struct cpfl_flow_js_pr_key_proto *key_proto;
> + enum rte_flow_item_type type;
> +
> + key_proto = &protocols[i];
> + /* pr->key->proto->type */
> + type = key_proto->type;
> + /* pr->key->proto->fields */
> + switch (type) {
> + case RTE_FLOW_ITEM_TYPE_ETH:
> + if (items[j++].type == RTE_FLOW_ITEM_TYPE_ETH) {
> + const struct rte_flow_item_eth *eth_mask;
> + int ret;
> +
> + eth_mask = (const struct rte_flow_item_eth
> *)items[i].mask;
> + ret = cpfl_check_eth(key_proto, eth_mask);
> + if (ret < 0)
> + return ret;
> + } else {
> + return -EINVAL;
> + }
> + break;
<...>
> +
> +
> +/* output: uint8_t *buffer, uint16_t *byte_len */
> +static int
> +cpfl_parse_layout(struct cpfl_flow_js_mr_layout *layouts, int layout_size,
> + struct cpfl_flow_mr_key_action *mr_key_action,
> + uint8_t *buffer, uint16_t *byte_len)
> +{
> + int i, start = 0;
int start = 0;
int i;
> +
> + for (i = 0; i < layout_size; i++) {
> + int index, size, offset;
> + const char *hint;
> + const uint8_t *addr;
> + struct cpfl_flow_mr_key_action *temp;
> + struct cpfl_flow_js_mr_layout *layout;
> +
> + layout = &layouts[i];
> + /* index links to the element of the actions array. */
> + index = layout->index;
> + size = layout->size;
> + offset = layout->offset;
> + if (index == -1) {
> + hint = "dummpy";
> + start += size;
> + continue;
> + }
> + hint = layout->hint;
> + addr = NULL;
> + temp = mr_key_action + index;
> +
> + if (temp->type == RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP) {
> + const struct rte_flow_action_vxlan_encap
> *action_vxlan_encap;
> + struct rte_flow_item *definition;
> + int def_length, k;
> +
> + action_vxlan_encap =
> + (const struct rte_flow_action_vxlan_encap *)temp-
> >encap.action->conf;
> + definition = action_vxlan_encap->definition;
> + def_length = cpfl_get_items_length(definition);
> + for (k = 0; k < def_length - 1; k++) {
> + if ((strcmp(hint, "eth") == 0 &&
> + definition[k].type ==
> RTE_FLOW_ITEM_TYPE_ETH) ||
> + (strcmp(hint, "ipv4") == 0 &&
> + definition[k].type ==
> RTE_FLOW_ITEM_TYPE_IPV4) ||
> + (strcmp(hint, "udp") == 0 &&
> + definition[k].type ==
> RTE_FLOW_ITEM_TYPE_UDP) ||
> + (strcmp(hint, "tcp") == 0 &&
> + definition[k].type ==
> RTE_FLOW_ITEM_TYPE_TCP) ||
> + (strcmp(hint, "vxlan") == 0 &&
> + definition[k].type ==
> RTE_FLOW_ITEM_TYPE_VXLAN)) {
> + addr = (const uint8_t
> *)(definition[k].spec);
> + if (start > 255) {
Better to use macro for 255.
> + *byte_len = 0;
> + PMD_DRV_LOG(ERR, "byte
> length is too long%s",
> + hint);
> + return -EINVAL;
> + }
> + memcpy(buffer + start, addr + offset,
> size);
> + break;
> + } /* TODO: more hint... */
> + }
> + if (k == def_length - 1) {
> + *byte_len = 0;
> + PMD_DRV_LOG(ERR, "can not find
> corresponding hint: %s", hint);
> + return -EINVAL;
> + }
> + } else {
> + *byte_len = 0;
> + PMD_DRV_LOG(ERR, "Not support this type: %d.",
> temp->type);
> + return -EINVAL;
> + }
> + /* else TODO: more type... */
> +
> + start += size;
> + }
> + *byte_len = start;
> + return 0;
> +}
> +
<...>
> diff --git a/drivers/net/cpfl/cpfl_flow_parser.h
> b/drivers/net/cpfl/cpfl_flow_parser.h
> new file mode 100644
> index 0000000000..af33051ce2
> --- /dev/null
> +++ b/drivers/net/cpfl/cpfl_flow_parser.h
> @@ -0,0 +1,205 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2023 Intel Corporation
> + */
> +#include <json-c/json.h>
> +#include <rte_flow.h>
> +
> +#ifndef _CPFL_FLOW_PARSER_H_
> +#define _CPFL_FLOW_PARSER_H_
> +
> +#define CPFL_FLOW_JSON_STR_SIZE_MAX 100
> +
> +/* Pattern Rules Storage Begin*/
> +enum cpfl_flow_pr_action_type {
> + CPFL_JS_PR_ACTION_TYPE_SEM,
> + CPFL_JS_PR_ACTION_TYPE_UNKNOWN = -1,
> +};
> +
> +struct cpfl_flow_js_pr_key_attr {
> + uint16_t ingress;
> + uint16_t egress;
> +};
> +
> +struct cpfl_flow_js_pr_key_proto_field {
> + char name[CPFL_FLOW_JSON_STR_SIZE_MAX];
> + union {
> + char mask[CPFL_FLOW_JSON_STR_SIZE_MAX];
> + uint32_t mask_32b;
> + };
> +};
> +
> +struct cpfl_flow_js_pr_key_proto {
> + enum rte_flow_item_type type;
> + struct cpfl_flow_js_pr_key_proto_field *fields;
> + int fields_size;
> +};
> +
> +enum cpfl_flow_js_fv_type {
> + CPFL_FV_TYPE_PROTOCOL,
> + CPFL_FV_TYPE_IMMEDIATE,
> + CPFL_FV_TYPE_UNKNOWN = -1,
> +
No need the blank line.
Could you add some comments for the type?
> +};
> +
> +struct cpfl_flow_js_fv {
> + uint16_t offset;
> + enum cpfl_flow_js_fv_type type;
> + union {
> + uint16_t immediate;
> + struct {
> + uint16_t layer;
> + enum rte_flow_item_type header;
> + uint16_t offset;
> + uint16_t mask;
> + } proto;
> + };
> +};
> +
> +#define CPFL_MAX_SEM_FV_KEY_SIZE 64
Move all macros up with CPFL_FLOW_JSON_STR_SIZE_MAX.
<...>
next prev parent reply other threads:[~2023-08-24 7:25 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-11 9:30 [PATCH 0/4] net/cpfl: add basic support for rte_flow Wenjing Qiao
2023-08-11 9:30 ` [PATCH 1/4] net/cpfl: parse flow parser file in devargs Wenjing Qiao
2023-08-11 10:00 ` [PATCH v2 0/4] net/cpfl: add basic support for rte_flow Wenjing Qiao
2023-08-11 10:00 ` [PATCH v2 1/4] net/cpfl: parse flow parser file in devargs Wenjing Qiao
2023-08-24 3:15 ` Xing, Beilei
2023-08-11 10:00 ` [PATCH v2 2/4] net/cpfl: add flow json parser Wenjing Qiao
2023-08-24 7:25 ` Xing, Beilei [this message]
2023-08-11 10:00 ` [PATCH v2 3/4] net/cpfl: introduce CPF common library Wenjing Qiao
2023-08-11 10:00 ` [PATCH v2 4/4] net/cpfl: setup ctrl path Wenjing Qiao
2023-08-11 9:30 ` [PATCH 2/4] net/cpfl: add flow json parser Wenjing Qiao
2023-08-11 9:30 ` [PATCH 3/4] net/cpfl: introduce CPF common library Wenjing Qiao
2023-08-24 9:19 ` Xing, Beilei
2023-09-01 8:14 ` Qiao, Wenjing
2023-08-11 9:30 ` [PATCH 4/4] net/cpfl: setup ctrl path Wenjing Qiao
2023-08-24 9:15 ` Xing, Beilei
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=LV2PR11MB5997AA43C96C72D533EC0E37F71DA@LV2PR11MB5997.namprd11.prod.outlook.com \
--to=beilei.xing@intel.com \
--cc=dev@dpdk.org \
--cc=mingxia.liu@intel.com \
--cc=wenjing.qiao@intel.com \
--cc=yuying.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).