On Tue, 20-01-14, 19:30, Jerin Jacob wrote:
> On Tue, Jan 14, 2020 at 3:45 PM Jack Min <jackmin@mellanox.com> wrote:
> >
> > On Tue, 20-01-14, 10:01, Jerin Jacob wrote:
> > > On Tue, Jan 14, 2020 at 9:15 AM Xiaoyu Min <jackmin@mellanox.com> wrote:
> > > >
> > > > From: Xueming Li <xuemingl@mellanox.com>
> > > >
> > > > New flow dump CLI to dump MLX5 PMD specific flows into screen.
> > > >
> > > > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> > > > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> > > > ---
> > > > app/test-pmd/Makefile | 4 ++
> > > > app/test-pmd/cmdline_flow.c | 91 +++++++++++++++++++++++++++++++++++++
> > > > app/test-pmd/config.c | 33 ++++++++++++++
> > > > app/test-pmd/meson.build | 3 ++
> > > > app/test-pmd/testpmd.h | 1 +
> > > > 5 files changed, 132 insertions(+)
> > > >
> > >
> > > >
> > > > +/** Dump all flow rules. */
> > > > +int
> > > > +port_flow_dump(portid_t port_id __rte_unused,
> > > > + const char *file_name __rte_unused)
> > > > +{
> > > > + int ret = 0;
> > > > +#ifdef RTE_LIBRTE_MLX5_PMD
> > >
> > > IMO, It should be the last resort to add driver-specific symbols in testpmd.
> > > Why not introduce rte_flow_dump() and hook the MLX driver underneath?
> > >
> > Hey Jerin,
>
> Hi Jack.
>
> >
> > Thanks for you comments.
> >
> > What my understanding is this flow dump is very Mellanox specific, it will dump
> > all flows in Mellanox HW using Mellanox format. They are hardware flows in
> > short, which are different from rte flow.
>
> We do have a similar API for other drivers to represent the internal info.
> I think, similar API[1] would suffice for your case. We should not
> standardize the output of dump, instead, it will provide a generic API
> to dump the internal representation of flow HW to file/stdout.
>
> Octeontx2 MACM HW (rte_flow) has similar internal information,
> We could implement the driver API if it is a generic API.
Yes, a generic rte_flow API (as similar as [1])can benifit both of us. :-)
I'll propose a new rte_flow API for this in v2.
Thanks,
-Jack
>
> [1]
> See the below example for eventdev driver.
>
> /**
> * Dump internal information about *dev_id* to the FILE* provided in *f*.
> *
> * @param dev_id
> * The identifier of the device.
> *
> * @param f
> * A pointer to a file for output
> *
> * @return
> * - 0: on success
> * - <0: on failure.
> */
> int
> rte_event_dev_dump(uint8_t dev_id, FILE *f);
>
>
> >
> > I don't know whether other vendor has the similar functionality and needs
> > so an rte flow level API could be helpful.
> >
> > And basically rte flow API is based on rte_flow, a dump function probabily means
> > dump rte_flow itself (i.e flow->attr, pattern, actions).
> >
> > This is the reason a private API is choosen and driver-specific symbols added
> > in testpmd as result.
> >
> > -Jack
@@ -70,6 +70,10 @@ ifeq ($(CONFIG_RTE_LIBRTE_PMD_SOFTNIC),y)
LDLIBS += -lrte_pmd_softnic
endif
+ifeq ($(CONFIG_RTE_LIBRTE_MLX5_PMD),y)
+LDLIBS += -lrte_pmd_mlx5
+endif
+
endif
include $(RTE_SDK)/mk/rte.app.mk
@@ -41,6 +41,7 @@ enum index {
BOOLEAN,
STRING,
HEX,
+ FILE_PATH,
MAC_ADDR,
IPV4_ADDR,
IPV6_ADDR,
@@ -63,6 +64,7 @@ enum index {
CREATE,
DESTROY,
FLUSH,
+ DUMP,
QUERY,
LIST,
ISOLATE,
@@ -631,6 +633,9 @@ struct buffer {
uint32_t *rule;
uint32_t rule_n;
} destroy; /**< Destroy arguments. */
+ struct {
+ char file[128];
+ } dump; /**< Dump arguments. */
struct {
uint32_t rule;
struct rte_flow_action action;
@@ -685,6 +690,12 @@ static const enum index next_destroy_attr[] = {
ZERO,
};
+static const enum index next_dump_attr[] = {
+ FILE_PATH,
+ END,
+ ZERO,
+};
+
static const enum index next_list_attr[] = {
LIST_GROUP,
END,
@@ -1374,6 +1385,9 @@ static int parse_destroy(struct context *, const struct token *,
static int parse_flush(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
+static int parse_dump(struct context *, const struct token *,
+ const char *, unsigned int,
+ void *, unsigned int);
static int parse_query(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
@@ -1401,6 +1415,9 @@ static int parse_string(struct context *, const struct token *,
static int parse_hex(struct context *ctx, const struct token *token,
const char *str, unsigned int len,
void *buf, unsigned int size);
+static int parse_string0(struct context *, const struct token *,
+ const char *, unsigned int,
+ void *, unsigned int);
static int parse_mac_addr(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
@@ -1494,6 +1511,12 @@ static const struct token token_list[] = {
.type = "HEX",
.help = "fixed string",
.call = parse_hex,
+ },
+ [FILE_PATH] = {
+ .name = "{file path}",
+ .type = "STRING",
+ .help = "file path",
+ .call = parse_string0,
.comp = comp_none,
},
[MAC_ADDR] = {
@@ -1555,6 +1578,7 @@ static const struct token token_list[] = {
CREATE,
DESTROY,
FLUSH,
+ DUMP,
LIST,
QUERY,
ISOLATE)),
@@ -1589,6 +1613,14 @@ static const struct token token_list[] = {
.args = ARGS(ARGS_ENTRY(struct buffer, port)),
.call = parse_flush,
},
+ [DUMP] = {
+ .name = "dump",
+ .help = "dump all flow rules to file",
+ .next = NEXT(next_dump_attr, NEXT_ENTRY(PORT_ID)),
+ .args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
+ ARGS_ENTRY(struct buffer, port)),
+ .call = parse_dump,
+ },
[QUERY] = {
.name = "query",
.help = "query an existing flow rule",
@@ -5012,6 +5044,33 @@ parse_flush(struct context *ctx, const struct token *token,
return len;
}
+/** Parse tokens for dump command. */
+static int
+parse_dump(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size)
+{
+ struct buffer *out = buf;
+
+ /* Token name must match. */
+ if (parse_default(ctx, token, str, len, NULL, 0) < 0)
+ return -1;
+ /* Nothing else to do if there is no buffer. */
+ if (!out)
+ return len;
+ if (!out->command) {
+ if (ctx->curr != DUMP)
+ return -1;
+ if (sizeof(*out) > size)
+ return -1;
+ out->command = ctx->curr;
+ ctx->objdata = 0;
+ ctx->object = out;
+ ctx->objmask = NULL;
+ }
+ return len;
+}
+
/** Parse tokens for query command. */
static int
parse_query(struct context *ctx, const struct token *token,
@@ -5409,6 +5468,35 @@ parse_hex(struct context *ctx, const struct token *token,
}
+/**
+ * Parse a zero-ended string.
+ */
+static int
+parse_string0(struct context *ctx, const struct token *token __rte_unused,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size)
+{
+ const struct arg *arg_data = pop_args(ctx);
+
+ /* Arguments are expected. */
+ if (!arg_data)
+ return -1;
+ size = arg_data->size;
+ /* Bit-mask fill is not supported. */
+ if (arg_data->mask || size < len + 1)
+ goto error;
+ if (!ctx->object)
+ return len;
+ buf = (uint8_t *)ctx->object + arg_data->offset;
+ strncpy(buf, str, len);
+ if (ctx->objmask)
+ memset((uint8_t *)ctx->objmask + arg_data->offset, 0xff, len);
+ return len;
+error:
+ push_args(ctx, arg_data);
+ return -1;
+}
+
/**
* Parse a MAC address.
*
@@ -6068,6 +6156,9 @@ cmd_flow_parsed(const struct buffer *in)
case FLUSH:
port_flow_flush(in->port);
break;
+ case DUMP:
+ port_flow_dump(in->port, in->args.dump.file);
+ break;
case QUERY:
port_flow_query(in->port, in->args.query.rule,
&in->args.query.action);
@@ -48,6 +48,9 @@
#ifdef RTE_LIBRTE_BNXT_PMD
#include <rte_pmd_bnxt.h>
#endif
+#ifdef RTE_LIBRTE_MLX5_PMD
+#include <rte_pmd_mlx5.h>
+#endif
#include <rte_gro.h>
#include <rte_config.h>
@@ -1441,6 +1444,36 @@ port_flow_flush(portid_t port_id)
return ret;
}
+/** Dump all flow rules. */
+int
+port_flow_dump(portid_t port_id __rte_unused,
+ const char *file_name __rte_unused)
+{
+ int ret = 0;
+#ifdef RTE_LIBRTE_MLX5_PMD
+ FILE * file = stdout;
+
+ if (file_name && strlen(file_name)) {
+ file = fopen(file_name, "w");
+ if (!file) {
+ printf("Failed to create file %s: %s\n", file_name,
+ strerror(errno));
+ return -errno;
+ }
+ }
+ ret = rte_pmd_mlx5_flow_dump(port_id, file);
+ if (ret)
+ printf("Failed to dump flow: %s\n", strerror(-ret));
+ else
+ printf("Flow dump finished\n");
+ if (file_name && strlen(file_name))
+ fclose(file);
+#else
+ printf("MLX5 PMD driver disabled\n");
+#endif
+ return ret;
+}
+
/** Query a flow rule. */
int
port_flow_query(portid_t port_id, uint32_t rule,
@@ -48,3 +48,6 @@ if dpdk_conf.has('RTE_LIBRTE_BPF')
sources += files('bpf_cmd.c')
deps += 'bpf'
endif
+if dpdk_conf.has('RTE_LIBRTE_MLX5_PMD')
+ deps += 'pmd_mlx5'
+endif
@@ -734,6 +734,7 @@ int port_flow_create(portid_t port_id,
const struct rte_flow_action *actions);
int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
int port_flow_flush(portid_t port_id);
+int port_flow_dump(portid_t port_id, const char *file_name);
int port_flow_query(portid_t port_id, uint32_t rule,
const struct rte_flow_action *action);
void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group);