I agree, if the proposed solution is accepted I'll upload a new patch with the rest of the fields of pattern and actions.
Regards, Asaf Penso > -----Original Message----- > From: Ori Kam > Sent: Tuesday, December 11, 2018 9:30 AM > To: Asaf Penso <as...@mellanox.com>; Stephen Hemminger > <step...@networkplumber.org> > Cc: Adrien Mazarguil <adrien.mazarg...@6wind.com>; dev@dpdk.org; > Shahaf Shuler <shah...@mellanox.com>; Thomas Monjalon > <tho...@monjalon.net> > Subject: RE: [dpdk-dev] [PATCH] ethdev: add function to print a flow > > General comment for the discussion. > > I think it is important that all fields of all actions and items will be > printed. > This means that any modification to the rte_flow should also reflect in this > function. > > Best, > Ori > > > > > -----Original Message----- > > From: Asaf Penso > > Sent: Tuesday, December 11, 2018 9:25 AM > > To: Stephen Hemminger <step...@networkplumber.org> > > Cc: Adrien Mazarguil <adrien.mazarg...@6wind.com>; dev@dpdk.org; > > Shahaf Shuler <shah...@mellanox.com>; Ori Kam <or...@mellanox.com>; > > Thomas Monjalon <tho...@monjalon.net> > > Subject: RE: [dpdk-dev] [PATCH] ethdev: add function to print a flow > > > > > > > > Regards, > > Asaf Penso > > > > > -----Original Message----- > > > From: Stephen Hemminger <step...@networkplumber.org> > > > Sent: Thursday, December 6, 2018 11:44 PM > > > To: Asaf Penso <as...@mellanox.com> > > > Cc: Adrien Mazarguil <adrien.mazarg...@6wind.com>; dev@dpdk.org; > > > Shahaf Shuler <shah...@mellanox.com>; Ori Kam > <or...@mellanox.com>; > > > Thomas Monjalon <tho...@monjalon.net> > > > Subject: Re: [dpdk-dev] [PATCH] ethdev: add function to print a flow > > > > > > On Wed, 28 Nov 2018 15:26:06 +0000 > > > Asaf Penso <as...@mellanox.com> wrote: > > > > > > > Flow contains the following information: port id, attributes, > > > > patterns and actions. > > > > The function rte_flow_print prints all above information. > > > > > > > > It can be used for debugging purposes to validate the behavior of > > > > different dpdk applications. > > > > > > > > Example: running testpmd with the following flow create: > > > > flow create 1 transfer ingress > > > > pattern eth src is 52:54:00:15:b1:b1 dst is 24:8A:07:8D:AE:C6 / > > > > end actions of_push_vlan ethertype 0x8100 / of_set_vlan_vid > > > > vlan_vid 0x888 / of_set_vlan_pcp vlan_pcp 7 / port_id id 0 / end > > > > Will result in this > > > > output: > > > > Print flow info > > > > port_id =1 > > > > group =0 > > > > priority =0 > > > > ingress =1 > > > > egress =0 > > > > transfer =1 > > > > group =0 > > > > reserved =0 > > > > pattern type =9 > > > > name=RTE_FLOW_ITEM_TYPE_ETH > > > > spec type=0x0, src=52:54:00:15:b1:b1, dst=24:8a:07:8d:ae:c6 > > > > mask type=0x0, src=ff:ff:ff:ff:ff:ff, dst=ff:ff:ff:ff:ff:ff > > > > actions type =23 name=RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN > > > > ethertype=0x81 > > > > actions type =24 > > > > name=RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID > > > > vlan_vid=0x8808 > > > > actions type =25 > > > > name=RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP > > > > vlan_pcp=7 > > > > actions type =13 > > > > name=RTE_FLOW_ACTION_TYPE_PORT_ID > > > > id=0 > > > > reserved=0 > > > > > > > > Signed-off-by: Asaf Penso <as...@mellanox.com> > > > > --- > > > > lib/librte_ethdev/rte_ethdev_version.map | 1 + > > > > lib/librte_ethdev/rte_flow.c | 226 > > > ++++++++++++++++++++++++++++++ > > > > lib/librte_ethdev/rte_flow.h | 31 ++++ > > > > 3 files changed, 258 insertions(+), 0 deletions(-) > > > > > > > > diff --git a/lib/librte_ethdev/rte_ethdev_version.map > > > > b/lib/librte_ethdev/rte_ethdev_version.map > > > > index 92ac3de..7676983 100644 > > > > --- a/lib/librte_ethdev/rte_ethdev_version.map > > > > +++ b/lib/librte_ethdev/rte_ethdev_version.map > > > > @@ -249,6 +249,7 @@ EXPERIMENTAL { > > > > rte_eth_switch_domain_free; > > > > rte_flow_conv; > > > > rte_flow_expand_rss; > > > > + rte_flow_print; > > > > rte_mtr_capabilities_get; > > > > rte_mtr_create; > > > > rte_mtr_destroy; > > > > diff --git a/lib/librte_ethdev/rte_flow.c > > > > b/lib/librte_ethdev/rte_flow.c index 3277be1..742d892 100644 > > > > --- a/lib/librte_ethdev/rte_flow.c > > > > +++ b/lib/librte_ethdev/rte_flow.c > > > > @@ -16,6 +16,8 @@ > > > > #include "rte_flow_driver.h" > > > > #include "rte_flow.h" > > > > > > > > +int rte_flow_logtype; > > > > + > > > > /** > > > > * Flow elements description tables. > > > > */ > > > > @@ -202,6 +204,222 @@ struct rte_flow_desc_data { > > > > NULL, rte_strerror(ENOSYS)); } > > > > > > > > +/* Example: > > > > + * > > > > + * struct eth_addr mac; > > > > + * [...] > > > > + * printf("The Ethernet address is "RTE_ETH_ADDR_FMT"\n", > > > > + * RTE_ETH_ADDR_ARGS(mac)); > > > > + * > > > > + */ > > > > +#define RTE_ETH_ADDR_FMT \ > > > > + > > > "%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"P > > > RIx8 > > > > +#define RTE_ETH_ADDR_ARGS(addr) > > > RTE_ETH_ADDR_BYTES_ARGS((addr).ea) > > > > +#define RTE_ETH_ADDR_BYTES_ARGS(bytes) \ > > > > + (bytes)[0], (bytes)[1], (bytes)[2], (bytes)[3], (bytes)[4], > > > > +(bytes)[5] > > > > + > > > > +/* Print the flow fields. */ > > > > +void __rte_experimental > > > > +rte_flow_print(uint16_t port_id, > > > > + const struct rte_flow_attr *attr, > > > > + const struct rte_flow_item pattern[], > > > > + const struct rte_flow_action actions[]) { > > > > + RTE_FLOW_LOG(INFO, "Print flow info\n"); > > > > + RTE_FLOW_LOG(INFO, "port_id =%u\n", port_id); > > > > + RTE_FLOW_LOG(INFO, "group =%u\n", attr->group); > > > > + RTE_FLOW_LOG(INFO, "priority =%u\n", attr->priority); > > > > + RTE_FLOW_LOG(INFO, "ingress =%u\n", attr->ingress); > > > > + RTE_FLOW_LOG(INFO, "egress =%u\n", attr->egress); > > > > + RTE_FLOW_LOG(INFO, "transfer =%u\n", attr->transfer); > > > > + RTE_FLOW_LOG(INFO, "group =%u\n", attr->group); > > > > + RTE_FLOW_LOG(INFO, "reserved =%u\n", attr->reserved); > > > > + > > > > + for (; pattern->type != RTE_FLOW_ITEM_TYPE_END; pattern++) { > > > > + RTE_FLOW_LOG(INFO, "pattern type =%u\n", pattern- > > > >type); > > > > + switch (pattern->type) { > > > > + case RTE_FLOW_ITEM_TYPE_ETH: { > > > > + RTE_FLOW_LOG(INFO, " > > > name=RTE_FLOW_ITEM_TYPE_ETH\n"); > > > > + if (pattern->spec) { > > > > + const struct rte_flow_item_eth *spec = > > > > + pattern->spec; > > > > + RTE_FLOW_LOG(INFO, " spec type=0x%x, " > > > > + "src="RTE_ETH_ADDR_FMT", > > > dst="RTE_ETH_ADDR_FMT > > > > + "\n", > > > > + spec->type, > > > > + RTE_ETH_ADDR_BYTES_ARGS(spec- > > > >src.addr_bytes), > > > > + RTE_ETH_ADDR_BYTES_ARGS(spec- > > > >dst.addr_bytes)); > > > > + } > > > > + > > > > + if (pattern->mask) { > > > > + const struct rte_flow_item_eth *mask = > > > > + pattern->mask; > > > > + RTE_FLOW_LOG(INFO, " mask type=0x%x, " > > > > + "src="RTE_ETH_ADDR_FMT", > > > dst="RTE_ETH_ADDR_FMT > > > > + "\n", > > > > + mask->type, > > > > + RTE_ETH_ADDR_BYTES_ARGS(mask- > > > >src.addr_bytes), > > > > + RTE_ETH_ADDR_BYTES_ARGS(mask- > > > >dst.addr_bytes)); > > > > + } > > > > + break; > > > > + } > > > > + case RTE_FLOW_ITEM_TYPE_VLAN: { > > > > + RTE_FLOW_LOG(INFO, " > > > name=RTE_FLOW_ITEM_TYPE_VLAN\n"); > > > > + if (pattern->spec) { > > > > + const struct rte_flow_item_vlan *spec = > > > > + pattern->spec; > > > > + RTE_FLOW_LOG(INFO, " spec tci=0x%x\n", > > > > + spec->tci); > > > > + RTE_FLOW_LOG(INFO, " spec > > > inner_type=%u\n", > > > > + spec->inner_type); > > > > + } > > > > + if (pattern->mask) { > > > > + const struct rte_flow_item_vlan *mask = > > > > + pattern->mask; > > > > + RTE_FLOW_LOG(INFO, " mask tci=0x%x\n", > > > > + mask->tci); > > > > + RTE_FLOW_LOG(INFO, " mask > > > inner_type=%u\n", > > > > + mask->inner_type); > > > > + } > > > > + break; > > > > + } > > > > + case RTE_FLOW_ITEM_TYPE_IPV4: { > > > > + RTE_FLOW_LOG(INFO, " > > > name=RTE_FLOW_ITEM_TYPE_IPV4\n"); > > > > + if (pattern->spec) { > > > > + const struct rte_flow_item_ipv4 *spec = > > > > + pattern->spec; > > > > + RTE_FLOW_LOG(INFO, " spec > > > version_ihl=%u\n", > > > > + spec->hdr.version_ihl); > > > > + RTE_FLOW_LOG(INFO, " spec > > > type_of_service=%u\n", > > > > + > > > > spec->hdr.type_of_service); > > > > + RTE_FLOW_LOG(INFO, " spec > > > total_length=%u\n", > > > > + spec->hdr.total_length); > > > > + RTE_FLOW_LOG(INFO, " spec > > > packet_id=%u\n", > > > > + spec->hdr.packet_id); > > > > + RTE_FLOW_LOG(INFO, " spec > > > fragment_offset=%u\n", > > > > + > > > > spec->hdr.fragment_offset); > > > > + RTE_FLOW_LOG(INFO, " spec > > > time_to_live=%u\n", > > > > + spec->hdr.time_to_live); > > > > + RTE_FLOW_LOG(INFO, " spec > > > next_proto_id=%u\n", > > > > + > > > > spec->hdr.next_proto_id); > > > > + RTE_FLOW_LOG(INFO, " spec > > > hdr_checksum=0x%x\n", > > > > + spec->hdr.hdr_checksum); > > > > + RTE_FLOW_LOG(INFO, > > > > + " spec src_addr=%d.%d.%d.%d\n", > > > > + (spec->hdr.src_addr & > > > > 0x000000FF), > > > > + (spec->hdr.src_addr & > > > > 0x0000FF00) > > > >> 8, > > > > + (spec->hdr.src_addr & > > > > 0x00FF0000) > > > >> 16, > > > > + (spec->hdr.src_addr & > > > > 0xFF000000) > > > > + >> 24); > > > > + RTE_FLOW_LOG(INFO, > > > > + " spec dst_addr=%d.%d.%d.%d\n", > > > > + (spec->hdr.dst_addr & > > > > 0x000000FF), > > > > + (spec->hdr.dst_addr & > > > > 0x0000FF00) > > > >> 8, > > > > + (spec->hdr.dst_addr & > > > > 0x00FF0000) > > > >> 16, > > > > + (spec->hdr.dst_addr & > > > > 0xFF000000) > > > > + >> 24); > > > > + } > > > > + > > > > + if (pattern->mask) { > > > > + const struct rte_flow_item_ipv4 *mask = > > > > + pattern->mask; > > > > + RTE_FLOW_LOG(INFO, " mask > > > version_ihl=%u\n", > > > > + mask->hdr.version_ihl); > > > > + RTE_FLOW_LOG(INFO, " mask > > > type_of_service=%u\n", > > > > + > > > > mask->hdr.type_of_service); > > > > + RTE_FLOW_LOG(INFO, " mask > > > total_length=%u\n", > > > > + mask->hdr.total_length); > > > > + RTE_FLOW_LOG(INFO, " mask > > > packet_id=%u\n", > > > > + mask->hdr.packet_id); > > > > + RTE_FLOW_LOG(INFO, " mask > > > fragment_offset=%u\n", > > > > + > > > > mask->hdr.fragment_offset); > > > > + RTE_FLOW_LOG(INFO, " mask > > > time_to_live=%u\n", > > > > + mask->hdr.time_to_live); > > > > + RTE_FLOW_LOG(INFO, " mask > > > next_proto_id=%u\n", > > > > + > > > > mask->hdr.next_proto_id); > > > > + RTE_FLOW_LOG(INFO, " mask > > > hdr_checksum=0x%x\n", > > > > + mask->hdr.hdr_checksum); > > > > + RTE_FLOW_LOG(INFO, > > > > + " mask src_addr=%d.%d.%d.%d\n", > > > > + (mask->hdr.src_addr & > > > > 0x000000FF), > > > > + (mask->hdr.src_addr & > > > > 0x0000FF00) > > > >> 8, > > > > + (mask->hdr.src_addr & > > > > 0x00FF0000) > > > >> 16, > > > > + (mask->hdr.src_addr & > > > > 0xFF000000) > > > > + >> 24); > > > > + RTE_FLOW_LOG(INFO, > > > > + " mask dst_addr=%d.%d.%d.%d\n", > > > > + (mask->hdr.dst_addr & > > > > 0x000000FF), > > > > + (mask->hdr.dst_addr & > > > > 0x0000FF00) > > > >> 8, > > > > + (mask->hdr.dst_addr & > > > > 0x00FF0000) > > > >> 16, > > > > + (mask->hdr.dst_addr & > > > > 0xFF000000) > > > > + >> 24); > > > > + } > > > > + break; > > > > + } > > > > + default: > > > > + RTE_FLOW_LOG(INFO, "unfamiliar pattern item\n"); > > > > + } > > > > + } > > > > + > > > > + for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) { > > > > + RTE_FLOW_LOG(INFO, "actions type =%u\n", actions- > > > >type); > > > > + switch (actions->type) { > > > > + case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN: { > > > > + RTE_FLOW_LOG(INFO, > > > > + " > > > name=RTE_FLOW_ACTION_TYPE_OF_POP_VLAN\n"); > > > > + break; > > > > + } > > > > + case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN: { > > > > + RTE_FLOW_LOG(INFO, > > > > + " > > > name=RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN\n"); > > > > + if (actions->conf) { > > > > + const struct > > > > rte_flow_action_of_push_vlan > > > > + *conf = actions->conf; > > > > + RTE_FLOW_LOG(INFO, " > > > ethertype=0x%x\n", > > > > + conf->ethertype); > > > > + } > > > > + break; > > > > + } > > > > + case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID: { > > > > + RTE_FLOW_LOG(INFO, > > > > + " > > > name=RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID\n"); > > > > + if (actions->conf) { > > > > + const struct > > > rte_flow_action_of_set_vlan_vid > > > > + *conf = actions->conf; > > > > + RTE_FLOW_LOG(INFO, " vlan_vid=0x%x\n", > > > > + conf->vlan_vid); > > > > + } > > > > + break; > > > > + } > > > > + case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP: { > > > > + RTE_FLOW_LOG(INFO, > > > > + " > > > name=RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP\n"); > > > > + if (actions->conf) { > > > > + const struct > > > rte_flow_action_of_set_vlan_pcp > > > > + *conf = actions->conf; > > > > + RTE_FLOW_LOG(INFO, " vlan_pcp=%u\n", > > > > + conf->vlan_pcp); > > > > + } > > > > + break; > > > > + } > > > > + case RTE_FLOW_ACTION_TYPE_PORT_ID: { > > > > + RTE_FLOW_LOG(INFO, > > > > + " > > > name=RTE_FLOW_ACTION_TYPE_PORT_ID\n"); > > > > + if (actions->conf) { > > > > + const struct rte_flow_action_port_id > > > > + *conf = actions->conf; > > > > + RTE_FLOW_LOG(INFO, " id=%u\n", conf- > > > >id); > > > > + RTE_FLOW_LOG(INFO, " reserved=%u\n", > > > > + conf->reserved); > > > > + } > > > > + break; > > > > + } > > > > + default: > > > > + RTE_FLOW_LOG(INFO, "unfamiliar action item\n"); > > > > + } > > > > + } > > > > +} > > > > + > > > > /* Create a flow rule on a given port. */ struct rte_flow * > > > > rte_flow_create(uint16_t port_id, @@ -1001,3 +1219,11 @@ enum > > > > rte_flow_conv_item_spec_type { > > > > }; > > > > return lsize; > > > > } > > > > + > > > > +RTE_INIT(flow_init_log) > > > > +{ > > > > + rte_flow_logtype = rte_log_register("lib.flow"); > > > > + if (rte_flow_logtype >= 0) > > > > + rte_log_set_level(rte_flow_logtype, RTE_LOG_INFO); } > > > > + > > > > diff --git a/lib/librte_ethdev/rte_flow.h > > > > b/lib/librte_ethdev/rte_flow.h index c0fe879..0a7e70b 100644 > > > > --- a/lib/librte_ethdev/rte_flow.h > > > > +++ b/lib/librte_ethdev/rte_flow.h > > > > @@ -28,6 +28,12 @@ > > > > #include <rte_udp.h> > > > > #include <rte_byteorder.h> > > > > #include <rte_esp.h> > > > > +#include <rte_log.h> > > > > + > > > > +extern int rte_flow_logtype; > > > > + > > > > +#define RTE_FLOW_LOG(level, ...) \ > > > > + rte_log(RTE_LOG_ ## level, rte_flow_logtype, "" __VA_ARGS__) > > > > > > > > #ifdef __cplusplus > > > > extern "C" { > > > > @@ -2414,6 +2420,31 @@ enum rte_flow_conv_op { > > > > struct rte_flow_error *error); > > > > > > > > /** > > > > + * Print the flow fields. > > > > + * > > > > + * The flow contains the port id, different attributes, the > > > > +pattern's > > > > + * items and the action's items, which are all being printed. > > > > + * > > > > + * @b EXPERIMENTAL: this API may change without prior notice > > > > + * > > > > + * @param port_id > > > > + * Port identifier of Ethernet device. > > > > + * @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 action). > > > > + * > > > > + * @note not all enum values of patterns and actions are printed. > > > > + */ > > > > +void __rte_experimental > > > > +rte_flow_print(uint16_t port_id, > > > > + const struct rte_flow_attr *attr, > > > > + const struct rte_flow_item pattern[], > > > > + const struct rte_flow_action actions[]); > > > > + > > > > +/** > > > > * Create a flow rule on a given port. > > > > * > > > > * @param port_id > > > > > > Since this is a debug function, it would make more sense for it to > > > take a 'FILE *' > > > for output like the other debug functions do. That would make it > > > consistent with rte_pktmbuf_dump rte_event_dev_dump, > rte_mempool_dump, etc. > > > > > > Why not name it rte_flow_dump? > > > > Thanks for pointing this out. I'll consider renaming and make it accept a > > file. > > I'll wait for more comments first before uploading another version of the > patch.