| Message ID | 1589465498-68144-1-git-send-email-u9012063@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [ovs-dev] dpif-netdev: Add miniflow bits to dump-flows. | expand |
On 5/14/2020 7:11 AM, William Tu wrote: > The 'dpctl/dump-flows -m' only shows the number of 1-bit in the > miniflow map, the patch outputs additional miniflow bits after it. > The format will be > dp-extra-info:miniflow_bits(count_1bit(unit0):unit0, > count_1bit(unit1):unit1) > Example: > dp-extra-info:miniflow_bits(4:0x30c0000000000000,1:0x400) > > By searching the unique miniflow bits, we know the number of subtables, > and for earch subtables, the fields it matches on. > > Cc: Emma Finn <emma.finn@intel.com> > Cc: Ian Stokes <ian.stokes@intel.com> > Signed-off-by: William Tu <u9012063@gmail.com> > --- > lib/dpif-netdev.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 51c888501bdf..b618b07be0c8 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -3352,8 +3352,9 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd, > if (unit) { > ds_put_char(&extra_info, ','); > } > - ds_put_format(&extra_info, "%d", > - count_1bits(flow->cr.mask->mf.map.bits[unit])); > + ds_put_format(&extra_info, "%d:0x%llx", > + count_1bits(flow->cr.mask->mf.map.bits[unit]), > + flow->cr.mask->mf.map.bits[unit]); > } > ds_put_char(&extra_info, ')'); > flow->dp_extra_info = ds_steal_cstr(&extra_info); > I haven't seen any reply from Emma Finn or Ian Stokes so I looked at this and it seems fine to me. Acked-by: Greg Rose <gvrose8192@gmail.com>
> -----Original Message----- > From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of Gregory Rose > Sent: Thursday 4 June 2020 20:06 > To: William Tu <u9012063@gmail.com>; dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] dpif-netdev: Add miniflow bits to dump- > flows. > > > On 5/14/2020 7:11 AM, William Tu wrote: > > The 'dpctl/dump-flows -m' only shows the number of 1-bit in the > > miniflow map, the patch outputs additional miniflow bits after it. > > The format will be > > dp-extra-info:miniflow_bits(count_1bit(unit0):unit0, > > count_1bit(unit1):unit1) > > Example: > > dp-extra-info:miniflow_bits(4:0x30c0000000000000,1:0x400) > > > > By searching the unique miniflow bits, we know the number of > > subtables, and for earch subtables, the fields it matches on. > > > > Cc: Emma Finn <emma.finn@intel.com> > > Cc: Ian Stokes <ian.stokes@intel.com> > > Signed-off-by: William Tu <u9012063@gmail.com> > > --- > > lib/dpif-netdev.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index > > 51c888501bdf..b618b07be0c8 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -3352,8 +3352,9 @@ dp_netdev_flow_add(struct > dp_netdev_pmd_thread *pmd, > > if (unit) { > > ds_put_char(&extra_info, ','); > > } > > - ds_put_format(&extra_info, "%d", > > - count_1bits(flow->cr.mask->mf.map.bits[unit])); > > + ds_put_format(&extra_info, "%d:0x%llx", > > + count_1bits(flow->cr.mask->mf.map.bits[unit]), > > + flow->cr.mask->mf.map.bits[unit]); > > } > > ds_put_char(&extra_info, ')'); > > flow->dp_extra_info = ds_steal_cstr(&extra_info); > > > > I haven't seen any reply from Emma Finn or Ian Stokes so I looked at this and > it seems fine to me. > > Acked-by: Greg Rose <gvrose8192@gmail.com> > Looks Good. Tested-by: Emma Finn <emma.finn@intel.com> _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 5/14/20 4:11 PM, William Tu wrote: > The 'dpctl/dump-flows -m' only shows the number of 1-bit in the > miniflow map, the patch outputs additional miniflow bits after it. > The format will be > dp-extra-info:miniflow_bits(count_1bit(unit0):unit0, > count_1bit(unit1):unit1) > Example: > dp-extra-info:miniflow_bits(4:0x30c0000000000000,1:0x400) > > By searching the unique miniflow bits, we know the number of subtables, > and for earch subtables, the fields it matches on. Hi. Beside the curiosity what is the purpose of printing this information? How can it be used? Best regards, Ilya Maximets. > > Cc: Emma Finn <emma.finn@intel.com> > Cc: Ian Stokes <ian.stokes@intel.com> > Signed-off-by: William Tu <u9012063@gmail.com> > --- > lib/dpif-netdev.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 51c888501bdf..b618b07be0c8 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -3352,8 +3352,9 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd, > if (unit) { > ds_put_char(&extra_info, ','); > } > - ds_put_format(&extra_info, "%d", > - count_1bits(flow->cr.mask->mf.map.bits[unit])); > + ds_put_format(&extra_info, "%d:0x%llx", > + count_1bits(flow->cr.mask->mf.map.bits[unit]), > + flow->cr.mask->mf.map.bits[unit]); > } > ds_put_char(&extra_info, ')'); > flow->dp_extra_info = ds_steal_cstr(&extra_info); >
On Mon, Jun 8, 2020 at 7:01 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 5/14/20 4:11 PM, William Tu wrote: > > The 'dpctl/dump-flows -m' only shows the number of 1-bit in the > > miniflow map, the patch outputs additional miniflow bits after it. > > The format will be > > dp-extra-info:miniflow_bits(count_1bit(unit0):unit0, > > count_1bit(unit1):unit1) > > Example: > > dp-extra-info:miniflow_bits(4:0x30c0000000000000,1:0x400) > > > > By searching the unique miniflow bits, we know the number of subtables, > > and for earch subtables, the fields it matches on. > > Hi. > > Beside the curiosity what is the purpose of printing this information? > How can it be used? > So from the bitmap we can know which field in the 'struct flow' this subtable is matching on. And collecting all the bitmaps from dpctl/dump-flow, we can know which fields are used to match more frequently than others. William
On 6/8/20 5:36 PM, William Tu wrote: > On Mon, Jun 8, 2020 at 7:01 AM Ilya Maximets <i.maximets@ovn.org> wrote: >> >> On 5/14/20 4:11 PM, William Tu wrote: >>> The 'dpctl/dump-flows -m' only shows the number of 1-bit in the >>> miniflow map, the patch outputs additional miniflow bits after it. >>> The format will be >>> dp-extra-info:miniflow_bits(count_1bit(unit0):unit0, >>> count_1bit(unit1):unit1) >>> Example: >>> dp-extra-info:miniflow_bits(4:0x30c0000000000000,1:0x400) >>> >>> By searching the unique miniflow bits, we know the number of subtables, >>> and for earch subtables, the fields it matches on. >> >> Hi. >> >> Beside the curiosity what is the purpose of printing this information? >> How can it be used? >> > So from the bitmap we can know which field in the 'struct flow' this > subtable is matching on. And collecting all the bitmaps from dpctl/dump-flow, > we can know which fields are used to match more frequently than others. Don't you have all this information from the flow match?
> On 6/8/20 5:36 PM, William Tu wrote: > > On Mon, Jun 8, 2020 at 7:01 AM Ilya Maximets <i.maximets@ovn.org> wrote: > >> > >> On 5/14/20 4:11 PM, William Tu wrote: > >>> The 'dpctl/dump-flows -m' only shows the number of 1-bit in the > >>> miniflow map, the patch outputs additional miniflow bits after it. > >>> The format will be > >>> dp-extra-info:miniflow_bits(count_1bit(unit0):unit0, > >>> count_1bit(unit1):unit1) > >>> Example: > >>> dp-extra-info:miniflow_bits(4:0x30c0000000000000,1:0x400) > >>> > >>> By searching the unique miniflow bits, we know the number of subtables, > >>> and for earch subtables, the fields it matches on. > >> > >> Hi. > >> > >> Beside the curiosity what is the purpose of printing this information? > >> How can it be used? > >> > > So from the bitmap we can know which field in the 'struct flow' this > > subtable is matching on. And collecting all the bitmaps from dpctl/dump-flow, > > we can know which fields are used to match more frequently than others. > > Don't you have all this information from the flow match? Hi William, I haven't seen feedback to Ilyas point above. I guess from our side I was think the same as Ilya, if this is available already then do we need to duplicate here? It may make sense if OVS doesn't have a way to easily map the bits back to its "struct flow" name That being said I suppose it's a question of usability, could the above example be improved further for usability, so something like: Example: dp-extra-info:miniflow_bits(4: ether src, ether dst,ipv4,udp 1: udp) Regards Ian
On Wed, Oct 21, 2020 at 8:47 AM Stokes, Ian <ian.stokes@intel.com> wrote: > > > On 6/8/20 5:36 PM, William Tu wrote: > > > On Mon, Jun 8, 2020 at 7:01 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > >> > > >> On 5/14/20 4:11 PM, William Tu wrote: > > >>> The 'dpctl/dump-flows -m' only shows the number of 1-bit in the > > >>> miniflow map, the patch outputs additional miniflow bits after it. > > >>> The format will be > > >>> dp-extra-info:miniflow_bits(count_1bit(unit0):unit0, > > >>> count_1bit(unit1):unit1) > > >>> Example: > > >>> dp-extra-info:miniflow_bits(4:0x30c0000000000000,1:0x400) > > >>> > > >>> By searching the unique miniflow bits, we know the number of subtables, > > >>> and for earch subtables, the fields it matches on. > > >> > > >> Hi. > > >> > > >> Beside the curiosity what is the purpose of printing this information? > > >> How can it be used? > > >> > > > So from the bitmap we can know which field in the 'struct flow' this > > > subtable is matching on. And collecting all the bitmaps from dpctl/dump-flow, > > > we can know which fields are used to match more frequently than others. > > > > Don't you have all this information from the flow match? > > Hi William, > > I haven't seen feedback to Ilyas point above. I guess from our side I was think the same as Ilya, if this is available already then do we need to duplicate here? > Hi Ian and Ilya, Thanks for the feedback. I agree with you that it's not worth duplicating the information here. Please drop this patch. Thanks! William
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 51c888501bdf..b618b07be0c8 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -3352,8 +3352,9 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd, if (unit) { ds_put_char(&extra_info, ','); } - ds_put_format(&extra_info, "%d", - count_1bits(flow->cr.mask->mf.map.bits[unit])); + ds_put_format(&extra_info, "%d:0x%llx", + count_1bits(flow->cr.mask->mf.map.bits[unit]), + flow->cr.mask->mf.map.bits[unit]); } ds_put_char(&extra_info, ')'); flow->dp_extra_info = ds_steal_cstr(&extra_info);
The 'dpctl/dump-flows -m' only shows the number of 1-bit in the miniflow map, the patch outputs additional miniflow bits after it. The format will be dp-extra-info:miniflow_bits(count_1bit(unit0):unit0, count_1bit(unit1):unit1) Example: dp-extra-info:miniflow_bits(4:0x30c0000000000000,1:0x400) By searching the unique miniflow bits, we know the number of subtables, and for earch subtables, the fields it matches on. Cc: Emma Finn <emma.finn@intel.com> Cc: Ian Stokes <ian.stokes@intel.com> Signed-off-by: William Tu <u9012063@gmail.com> --- lib/dpif-netdev.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)