| Message ID | 1579175088-45449-1-git-send-email-emma.finn@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v4] dpif-netdev: Modified ovs-appctl dpctl/dump-flows command | expand |
Comments inline. > -----Original Message----- > From: Finn, Emma <emma.finn@intel.com> > Sent: Thursday 16 January 2020 11:45 > To: dev@openvswitch.org > Cc: i.maximets@ovn.org; ian.stokes@intel.org; Finn, Emma > <emma.finn@intel.com> > Subject: [v4] dpif-netdev: Modified ovs-appctl dpctl/dump-flows command > > Modified ovs-appctl dpctl/dump-flows command to output the miniflow bits > for a given flow when -m option is passed. > > $ ovs-appctl dpctl/dump-flows -m > > Signed-off-by: Emma Finn <emma.finn@intel.com> > > --- > > RFC -> v1 > > * Changed revision from RFC to v1 > * Reformatted based on comments > * Fixed same classifier being dumped multiple times > flagged by Ilya > * Fixed print of subtables flagged by William > * Updated print count of bits as well as bits themselves > > --- > > v1 -> v2 > > * Reformatted based on comments > * Refactored code to make output part of ovs-appctl > dpctl/dump-flows -m command. > > --- > > v2 -> v3 > > * Added attribute dp_extra_info to dpif_flow_attrs struct > to store miniflow bits as a string > > --- > > v3 -> v4 > > * Fixed string leak > * Refactored to code to make it independent from the flowmap size > --- Any other feedback? I will have to rebase a send v5. Regards, Emma > NEWS | 2 ++ > lib/dpctl.c | 5 +++++ > lib/dpif-netdev.c | 14 ++++++++++++++ > lib/dpif.h | 1 + > 4 files changed, 22 insertions(+) > > diff --git a/NEWS b/NEWS > index 965faca..1c9d2db 100644 > --- a/NEWS > +++ b/NEWS > @@ -8,6 +8,8 @@ Post-v2.12.0 > * Add option to enable, disable and query TCP sequence checking in > conntrack. > * Add support for conntrack zone limits. > + * Command "ovs-appctl dpctl/dump-flows" refactored to show subtable > + miniflow bits for userspace datapath. > - AF_XDP: > * New option 'use-need-wakeup' for netdev-afxdp to control enabling > of corresponding 'need_wakeup' flag in AF_XDP rings. Enabled by > default diff --git a/lib/dpctl.c b/lib/dpctl.c index a1ea25b..1b0a2bf 100644 > --- a/lib/dpctl.c > +++ b/lib/dpctl.c > @@ -825,6 +825,11 @@ format_dpif_flow(struct ds *ds, const struct > dpif_flow *f, struct hmap *ports, > } > ds_put_cstr(ds, ", actions:"); > format_odp_actions(ds, f->actions, f->actions_len, ports); > + if (dpctl_p->verbosity && f->attrs.dp_extra_info) { > + ds_put_format(ds, ", dp-extra-info:%s", > + f->attrs.dp_extra_info); > + } > + free(f->attrs.dp_extra_info); > } > > struct dump_types { > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 079bd1b..a640b49 > 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -3101,6 +3101,20 @@ dp_netdev_flow_to_dpif_flow(const struct > dp_netdev_flow *netdev_flow, > > flow->attrs.offloaded = false; > flow->attrs.dp_layer = "ovs"; > + > + struct ds extra_info = DS_EMPTY_INITIALIZER; > + size_t unit; > + > + ds_put_cstr(&extra_info, "miniflow_bits("); FLOWMAP_FOR_EACH_UNIT > (unit) { > + if (unit) { > + ds_put_char(&extra_info, ','); > + } > + ds_put_format(&extra_info, "%d", > + count_1bits(netdev_flow->cr.mask->mf.map.bits[unit])); > + } > + ds_put_char(&extra_info, ')'); > + flow->attrs.dp_extra_info = ds_steal_cstr(&extra_info); > + ds_destroy(&extra_info); > } > > static int > diff --git a/lib/dpif.h b/lib/dpif.h > index c21e897..59d82dc 100644 > --- a/lib/dpif.h > +++ b/lib/dpif.h > @@ -513,6 +513,7 @@ struct dpif_flow_detailed_stats { struct > dpif_flow_attrs { > bool offloaded; /* True if flow is offloaded to HW. */ > const char *dp_layer; /* DP layer the flow is handled in. */ > + char *dp_extra_info; /* Extra information provided by DP. */ > }; > > struct dpif_flow_dump_types { > -- > 2.7.4
On 16.01.2020 15:09, Finn, Emma wrote: > Comments inline. > >> -----Original Message----- >> From: Finn, Emma <emma.finn@intel.com> >> Sent: Thursday 16 January 2020 11:45 >> To: dev@openvswitch.org >> Cc: i.maximets@ovn.org; ian.stokes@intel.org; Finn, Emma >> <emma.finn@intel.com> >> Subject: [v4] dpif-netdev: Modified ovs-appctl dpctl/dump-flows command >> >> Modified ovs-appctl dpctl/dump-flows command to output the miniflow bits >> for a given flow when -m option is passed. >> >> $ ovs-appctl dpctl/dump-flows -m >> >> Signed-off-by: Emma Finn <emma.finn@intel.com> >> >> --- >> >> RFC -> v1 >> >> * Changed revision from RFC to v1 >> * Reformatted based on comments >> * Fixed same classifier being dumped multiple times >> flagged by Ilya >> * Fixed print of subtables flagged by William >> * Updated print count of bits as well as bits themselves >> >> --- >> >> v1 -> v2 >> >> * Reformatted based on comments >> * Refactored code to make output part of ovs-appctl >> dpctl/dump-flows -m command. >> >> --- >> >> v2 -> v3 >> >> * Added attribute dp_extra_info to dpif_flow_attrs struct >> to store miniflow bits as a string >> >> --- >> >> v3 -> v4 >> >> * Fixed string leak >> * Refactored to code to make it independent from the flowmap size >> --- > > Any other feedback? > I will have to rebase a send v5. I didn't test this. But since you're rebasing, couple of style coments inline. > > Regards, > Emma > >> NEWS | 2 ++ >> lib/dpctl.c | 5 +++++ >> lib/dpif-netdev.c | 14 ++++++++++++++ >> lib/dpif.h | 1 + >> 4 files changed, 22 insertions(+) >> >> diff --git a/NEWS b/NEWS >> index 965faca..1c9d2db 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -8,6 +8,8 @@ Post-v2.12.0 >> * Add option to enable, disable and query TCP sequence checking in >> conntrack. >> * Add support for conntrack zone limits. >> + * Command "ovs-appctl dpctl/dump-flows" refactored to show subtable >> + miniflow bits for userspace datapath. >> - AF_XDP: >> * New option 'use-need-wakeup' for netdev-afxdp to control enabling >> of corresponding 'need_wakeup' flag in AF_XDP rings. Enabled by >> default diff --git a/lib/dpctl.c b/lib/dpctl.c index a1ea25b..1b0a2bf 100644 >> --- a/lib/dpctl.c >> +++ b/lib/dpctl.c >> @@ -825,6 +825,11 @@ format_dpif_flow(struct ds *ds, const struct >> dpif_flow *f, struct hmap *ports, >> } >> ds_put_cstr(ds, ", actions:"); >> format_odp_actions(ds, f->actions, f->actions_len, ports); >> + if (dpctl_p->verbosity && f->attrs.dp_extra_info) { >> + ds_put_format(ds, ", dp-extra-info:%s", >> + f->attrs.dp_extra_info); This should fit in a single line. Doesn't it? >> + } >> + free(f->attrs.dp_extra_info); >> } >> >> struct dump_types { >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 079bd1b..a640b49 >> 100644 >> --- a/lib/dpif-netdev.c >> +++ b/lib/dpif-netdev.c >> @@ -3101,6 +3101,20 @@ dp_netdev_flow_to_dpif_flow(const struct >> dp_netdev_flow *netdev_flow, >> >> flow->attrs.offloaded = false; >> flow->attrs.dp_layer = "ovs"; >> + >> + struct ds extra_info = DS_EMPTY_INITIALIZER; >> + size_t unit; >> + >> + ds_put_cstr(&extra_info, "miniflow_bits("); FLOWMAP_FOR_EACH_UNIT >> (unit) { Something bad happened here so lines was concatenated. >> + if (unit) { >> + ds_put_char(&extra_info, ','); >> + } >> + ds_put_format(&extra_info, "%d", >> + count_1bits(netdev_flow->cr.mask->mf.map.bits[unit])); >> + } >> + ds_put_char(&extra_info, ')'); >> + flow->attrs.dp_extra_info = ds_steal_cstr(&extra_info); >> + ds_destroy(&extra_info); >> } >> >> static int >> diff --git a/lib/dpif.h b/lib/dpif.h >> index c21e897..59d82dc 100644 >> --- a/lib/dpif.h >> +++ b/lib/dpif.h >> @@ -513,6 +513,7 @@ struct dpif_flow_detailed_stats { struct >> dpif_flow_attrs { >> bool offloaded; /* True if flow is offloaded to HW. */ >> const char *dp_layer; /* DP layer the flow is handled in. */ >> + char *dp_extra_info; /* Extra information provided by DP. */ >> }; >> >> struct dpif_flow_dump_types { >> -- >> 2.7.4 >
diff --git a/NEWS b/NEWS index 965faca..1c9d2db 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,8 @@ Post-v2.12.0 * Add option to enable, disable and query TCP sequence checking in conntrack. * Add support for conntrack zone limits. + * Command "ovs-appctl dpctl/dump-flows" refactored to show subtable + miniflow bits for userspace datapath. - AF_XDP: * New option 'use-need-wakeup' for netdev-afxdp to control enabling of corresponding 'need_wakeup' flag in AF_XDP rings. Enabled by default diff --git a/lib/dpctl.c b/lib/dpctl.c index a1ea25b..1b0a2bf 100644 --- a/lib/dpctl.c +++ b/lib/dpctl.c @@ -825,6 +825,11 @@ format_dpif_flow(struct ds *ds, const struct dpif_flow *f, struct hmap *ports, } ds_put_cstr(ds, ", actions:"); format_odp_actions(ds, f->actions, f->actions_len, ports); + if (dpctl_p->verbosity && f->attrs.dp_extra_info) { + ds_put_format(ds, ", dp-extra-info:%s", + f->attrs.dp_extra_info); + } + free(f->attrs.dp_extra_info); } struct dump_types { diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 079bd1b..a640b49 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -3101,6 +3101,20 @@ dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow *netdev_flow, flow->attrs.offloaded = false; flow->attrs.dp_layer = "ovs"; + + struct ds extra_info = DS_EMPTY_INITIALIZER; + size_t unit; + + ds_put_cstr(&extra_info, "miniflow_bits("); FLOWMAP_FOR_EACH_UNIT (unit) { + if (unit) { + ds_put_char(&extra_info, ','); + } + ds_put_format(&extra_info, "%d", + count_1bits(netdev_flow->cr.mask->mf.map.bits[unit])); + } + ds_put_char(&extra_info, ')'); + flow->attrs.dp_extra_info = ds_steal_cstr(&extra_info); + ds_destroy(&extra_info); } static int diff --git a/lib/dpif.h b/lib/dpif.h index c21e897..59d82dc 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -513,6 +513,7 @@ struct dpif_flow_detailed_stats { struct dpif_flow_attrs { bool offloaded; /* True if flow is offloaded to HW. */ const char *dp_layer; /* DP layer the flow is handled in. */ + char *dp_extra_info; /* Extra information provided by DP. */ }; struct dpif_flow_dump_types {
Modified ovs-appctl dpctl/dump-flows command to output the miniflow bits for a given flow when -m option is passed. $ ovs-appctl dpctl/dump-flows -m Signed-off-by: Emma Finn <emma.finn@intel.com> --- RFC -> v1 * Changed revision from RFC to v1 * Reformatted based on comments * Fixed same classifier being dumped multiple times flagged by Ilya * Fixed print of subtables flagged by William * Updated print count of bits as well as bits themselves --- v1 -> v2 * Reformatted based on comments * Refactored code to make output part of ovs-appctl dpctl/dump-flows -m command. --- v2 -> v3 * Added attribute dp_extra_info to dpif_flow_attrs struct to store miniflow bits as a string --- v3 -> v4 * Fixed string leak * Refactored to code to make it independent from the flowmap size --- NEWS | 2 ++ lib/dpctl.c | 5 +++++ lib/dpif-netdev.c | 14 ++++++++++++++ lib/dpif.h | 1 + 4 files changed, 22 insertions(+)