Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document subtable-lookup-* CMDs

2021-06-23 Thread Flavio Leitner
On Thu, Jun 17, 2021 at 05:18:21PM +0100, Cian Ferriter wrote:
> Signed-off-by: Cian Ferriter 
> 
> ---

Acked-by: Flavio Leitner 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document subtable-lookup-* CMDs

2021-06-22 Thread Stokes, Ian
> Hi all,
> 
> Thanks for the feedback. My responses are inline.
> 
> Cian
> 
> > -Original Message-
> > From: Flavio Leitner 
> > Sent: Tuesday 22 June 2021 17:39
> > To: Stokes, Ian 
> > Cc: Ferriter, Cian ; ovs-dev@openvswitch.org;
> i.maxim...@ovn.org
> > Subject: Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document
> subtable-lookup-* CMDs
> >
> > On Tue, Jun 22, 2021 at 03:42:46PM +, Stokes, Ian wrote:
> > > > Hi Flavio,
> > > >
> > > > Thanks for the review. My responses are inline.
> > > >
> > > > Cian
> > > >
> > > > > -Original Message-
> > > > > From: Flavio Leitner 
> > > > > Sent: Monday 21 June 2021 19:22
> > > > > To: Ferriter, Cian 
> > > > > Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org
> > > > > Subject: Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document
> > > > subtable-lookup-* CMDs
> > > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > This commit could be submitted outside of this patch-set as fix
> > > > > for commit 9ff7cabfd7 ("dpif-netdev: add subtable-lookup-prio-get
> > > > > command") and commit 3d018c3ea79d ("dpif-netdev: add subtable
> lookup
> > > > > prio set command.").
> > > > >
> > > > > This helps to get it merged sooner and reduce this patch-set size.
> > > > >
> > > >
> > > > I'll remove this patch from the patchset and send to the mailing list
> separately.
> > > > I'll wait till the DPIF patchset has been merged to send this, since I 
> > > > don't
> want
> > > > there to be rebase conflicts (the DPIF patchset also modifies this part 
> > > > of
> lib/dpif-
> > > > netdev-unixctl.man).
> > > >
> > > > I'll add the appropriate Fixes tags.
> > >
> > > @Flavio, If there is an aim to reduce the overall patch number of
> > > the series then I would recommend the following patches be
> > > submitted separately also from this series as there is no
> > > dependency on them to enable DPIF with AVX512.
> > >
> > > [v13 10/12] dpif-netdev/dpcls: Specialize more subtable signatures.
> > > [v13 11/12] dpdk: Cache result of CPU ISA checks.
> > >
> > >
> > > These two are quite small and I think could almost be applied now
> > > rather than as part of the series as they are modifying existing
> > > functionality (DPCLS AVX512 supported traffic types and DPDK flag
> > > caching).
> > >
> > > Thoughts?
> >
> > The idea is to get unrelated chunks merged sooner, if they make
> > sense of course, and then we have less patches to carry on.
> >
> > If the patches are going to arrive later or it causes more work
> > on following up patches, then I see no benefit.
> >
> > It's a suggestion. I am happy to review either way.
> >
> > Thanks,
> > fbl
> 
> I think splitting out the 2 above patches might cause a bit more work on our
> side. I'll leave them in the v14 patchset based on no strong preferences 
> above.
> 
> Since I've already taken out the "[v13 08/12] dpif-netdev-unixctl.man: 
> Document
> subtable-lookup-* CMDs" patch, I'll leave that out and submit separately.

That's OK with me, as I said they are not major patches and you could argue 
both sides whether they are logically connected.

Regards
Ian
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document subtable-lookup-* CMDs

2021-06-22 Thread Ferriter, Cian
Hi all,

Thanks for the feedback. My responses are inline.

Cian

> -Original Message-
> From: Flavio Leitner 
> Sent: Tuesday 22 June 2021 17:39
> To: Stokes, Ian 
> Cc: Ferriter, Cian ; ovs-dev@openvswitch.org; 
> i.maxim...@ovn.org
> Subject: Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document 
> subtable-lookup-* CMDs
> 
> On Tue, Jun 22, 2021 at 03:42:46PM +, Stokes, Ian wrote:
> > > Hi Flavio,
> > >
> > > Thanks for the review. My responses are inline.
> > >
> > > Cian
> > >
> > > > -Original Message-
> > > > From: Flavio Leitner 
> > > > Sent: Monday 21 June 2021 19:22
> > > > To: Ferriter, Cian 
> > > > Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org
> > > > Subject: Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document
> > > subtable-lookup-* CMDs
> > > >
> > > >
> > > > Hi,
> > > >
> > > > This commit could be submitted outside of this patch-set as fix
> > > > for commit 9ff7cabfd7 ("dpif-netdev: add subtable-lookup-prio-get
> > > > command") and commit 3d018c3ea79d ("dpif-netdev: add subtable lookup
> > > > prio set command.").
> > > >
> > > > This helps to get it merged sooner and reduce this patch-set size.
> > > >
> > >
> > > I'll remove this patch from the patchset and send to the mailing list 
> > > separately.
> > > I'll wait till the DPIF patchset has been merged to send this, since I 
> > > don't want
> > > there to be rebase conflicts (the DPIF patchset also modifies this part 
> > > of lib/dpif-
> > > netdev-unixctl.man).
> > >
> > > I'll add the appropriate Fixes tags.
> >
> > @Flavio, If there is an aim to reduce the overall patch number of
> > the series then I would recommend the following patches be
> > submitted separately also from this series as there is no
> > dependency on them to enable DPIF with AVX512.
> >
> > [v13 10/12] dpif-netdev/dpcls: Specialize more subtable signatures.
> > [v13 11/12] dpdk: Cache result of CPU ISA checks.
> >
> >
> > These two are quite small and I think could almost be applied now
> > rather than as part of the series as they are modifying existing
> > functionality (DPCLS AVX512 supported traffic types and DPDK flag
> > caching).
> >
> > Thoughts?
> 
> The idea is to get unrelated chunks merged sooner, if they make
> sense of course, and then we have less patches to carry on.
> 
> If the patches are going to arrive later or it causes more work
> on following up patches, then I see no benefit.
> 
> It's a suggestion. I am happy to review either way.
> 
> Thanks,
> fbl

I think splitting out the 2 above patches might cause a bit more work on our 
side. I'll leave them in the v14 patchset based on no strong preferences above.

Since I've already taken out the "[v13 08/12] dpif-netdev-unixctl.man: Document 
subtable-lookup-* CMDs" patch, I'll leave that out and submit separately.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document subtable-lookup-* CMDs

2021-06-22 Thread Flavio Leitner
On Tue, Jun 22, 2021 at 03:42:46PM +, Stokes, Ian wrote:
> > Hi Flavio,
> > 
> > Thanks for the review. My responses are inline.
> > 
> > Cian
> > 
> > > -Original Message-
> > > From: Flavio Leitner 
> > > Sent: Monday 21 June 2021 19:22
> > > To: Ferriter, Cian 
> > > Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org
> > > Subject: Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document
> > subtable-lookup-* CMDs
> > >
> > >
> > > Hi,
> > >
> > > This commit could be submitted outside of this patch-set as fix
> > > for commit 9ff7cabfd7 ("dpif-netdev: add subtable-lookup-prio-get
> > > command") and commit 3d018c3ea79d ("dpif-netdev: add subtable lookup
> > > prio set command.").
> > >
> > > This helps to get it merged sooner and reduce this patch-set size.
> > >
> > 
> > I'll remove this patch from the patchset and send to the mailing list 
> > separately.
> > I'll wait till the DPIF patchset has been merged to send this, since I 
> > don't want
> > there to be rebase conflicts (the DPIF patchset also modifies this part of 
> > lib/dpif-
> > netdev-unixctl.man).
> > 
> > I'll add the appropriate Fixes tags.
> 
> @Flavio, If there is an aim to reduce the overall patch number of
> the series then I would recommend the following patches be
> submitted separately also from this series as there is no
> dependency on them to enable DPIF with AVX512.
> 
> [v13 10/12] dpif-netdev/dpcls: Specialize more subtable signatures.
> [v13 11/12] dpdk: Cache result of CPU ISA checks.
> 
> 
> These two are quite small and I think could almost be applied now
> rather than as part of the series as they are modifying existing
> functionality (DPCLS AVX512 supported traffic types and DPDK flag
> caching).
> 
> Thoughts?

The idea is to get unrelated chunks merged sooner, if they make
sense of course, and then we have less patches to carry on. 

If the patches are going to arrive later or it causes more work
on following up patches, then I see no benefit.

It's a suggestion. I am happy to review either way.

Thanks,
fbl
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document subtable-lookup-* CMDs

2021-06-22 Thread Stokes, Ian
> Hi Flavio,
> 
> Thanks for the review. My responses are inline.
> 
> Cian
> 
> > -Original Message-
> > From: Flavio Leitner 
> > Sent: Monday 21 June 2021 19:22
> > To: Ferriter, Cian 
> > Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org
> > Subject: Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document
> subtable-lookup-* CMDs
> >
> >
> > Hi,
> >
> > This commit could be submitted outside of this patch-set as fix
> > for commit 9ff7cabfd7 ("dpif-netdev: add subtable-lookup-prio-get
> > command") and commit 3d018c3ea79d ("dpif-netdev: add subtable lookup
> > prio set command.").
> >
> > This helps to get it merged sooner and reduce this patch-set size.
> >
> 
> I'll remove this patch from the patchset and send to the mailing list 
> separately.
> I'll wait till the DPIF patchset has been merged to send this, since I don't 
> want
> there to be rebase conflicts (the DPIF patchset also modifies this part of 
> lib/dpif-
> netdev-unixctl.man).
> 
> I'll add the appropriate Fixes tags.

@Flavio, If there is an aim to reduce the overall patch number of the series 
then I would recommend the following patches be submitted separately also from 
this series as there is no dependency on them to enable DPIF with AVX512.

[v13 10/12] dpif-netdev/dpcls: Specialize more subtable signatures.
[v13 11/12] dpdk: Cache result of CPU ISA checks.


These two are quite small and I think could almost be applied now rather than 
as part of the series as they are modifying existing functionality (DPCLS 
AVX512 supported traffic types and DPDK flag caching).

Thoughts?

Regards
Ian



> 
> > Thanks for documenting it.
> > fbl
> >
> > On Thu, Jun 17, 2021 at 05:18:21PM +0100, Cian Ferriter wrote:
> > > Signed-off-by: Cian Ferriter 
> > >
> > > ---
> > >
> > > v13:
> > > - New commit to update manpages with more commands that are missing.
> > > ---
> > >  lib/dpif-netdev-unixctl.man | 10 ++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/lib/dpif-netdev-unixctl.man b/lib/dpif-netdev-unixctl.man
> > > index 45a1bd669..d77f5d9a4 100644
> > > --- a/lib/dpif-netdev-unixctl.man
> > > +++ b/lib/dpif-netdev-unixctl.man
> > > @@ -228,6 +228,16 @@ When this is the case, the above command prints
> the load-balancing
> > information
> > >  of the bonds configured in datapath \fIdp\fR showing the interface
> associated
> > >  with each bucket (hash).
> > >  .
> > > +.IP "\fBdpif-netdev/subtable-lookup-prio-get\fR"
> > > +Lists the DPCLS implementations or lookup functions that are available as
> well
> > > +as their priorities.
> > > +.
> > > +.IP "\fBdpif-netdev/subtable-lookup-prio-set\fR \fIlookup_function\fR \
> > > +\fIprio\fR"
> > > +Sets the priority of a lookup function by the name, 
> > > \fIlookup_function\fR,
> and
> > > +the priority, \fIprio\fR, which should be a positive integer value. The 
> > > highest
> > > +priority lookup function is used for classification.
> > > +.
> > >  .IP "\fBdpif-netdev/dpif-get\fR
> > >  Lists the DPIF implementations that are available.
> > >  .
> > > --
> > > 2.32.0
> > >
> > > ___
> > > dev mailing list
> > > d...@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> > --
> > fbl
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document subtable-lookup-* CMDs

2021-06-22 Thread Ferriter, Cian
Hi Flavio,

Thanks for the review. My responses are inline.

Cian

> -Original Message-
> From: Flavio Leitner 
> Sent: Monday 21 June 2021 19:22
> To: Ferriter, Cian 
> Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org
> Subject: Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document 
> subtable-lookup-* CMDs
> 
> 
> Hi,
> 
> This commit could be submitted outside of this patch-set as fix
> for commit 9ff7cabfd7 ("dpif-netdev: add subtable-lookup-prio-get
> command") and commit 3d018c3ea79d ("dpif-netdev: add subtable lookup
> prio set command.").
> 
> This helps to get it merged sooner and reduce this patch-set size.
> 

I'll remove this patch from the patchset and send to the mailing list 
separately.
I'll wait till the DPIF patchset has been merged to send this, since I don't 
want there to be rebase conflicts (the DPIF patchset also modifies this part of 
lib/dpif-netdev-unixctl.man).

I'll add the appropriate Fixes tags.

> Thanks for documenting it.
> fbl
> 
> On Thu, Jun 17, 2021 at 05:18:21PM +0100, Cian Ferriter wrote:
> > Signed-off-by: Cian Ferriter 
> >
> > ---
> >
> > v13:
> > - New commit to update manpages with more commands that are missing.
> > ---
> >  lib/dpif-netdev-unixctl.man | 10 ++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/lib/dpif-netdev-unixctl.man b/lib/dpif-netdev-unixctl.man
> > index 45a1bd669..d77f5d9a4 100644
> > --- a/lib/dpif-netdev-unixctl.man
> > +++ b/lib/dpif-netdev-unixctl.man
> > @@ -228,6 +228,16 @@ When this is the case, the above command prints the 
> > load-balancing
> information
> >  of the bonds configured in datapath \fIdp\fR showing the interface 
> > associated
> >  with each bucket (hash).
> >  .
> > +.IP "\fBdpif-netdev/subtable-lookup-prio-get\fR"
> > +Lists the DPCLS implementations or lookup functions that are available as 
> > well
> > +as their priorities.
> > +.
> > +.IP "\fBdpif-netdev/subtable-lookup-prio-set\fR \fIlookup_function\fR \
> > +\fIprio\fR"
> > +Sets the priority of a lookup function by the name, \fIlookup_function\fR, 
> > and
> > +the priority, \fIprio\fR, which should be a positive integer value. The 
> > highest
> > +priority lookup function is used for classification.
> > +.
> >  .IP "\fBdpif-netdev/dpif-get\fR
> >  Lists the DPIF implementations that are available.
> >  .
> > --
> > 2.32.0
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> --
> fbl
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [v13 08/12] dpif-netdev-unixctl.man: Document subtable-lookup-* CMDs

2021-06-21 Thread Flavio Leitner


Hi,

This commit could be submitted outside of this patch-set as fix
for commit 9ff7cabfd7 ("dpif-netdev: add subtable-lookup-prio-get
command") and commit 3d018c3ea79d ("dpif-netdev: add subtable lookup
prio set command.").

This helps to get it merged sooner and reduce this patch-set size.

Thanks for documenting it.
fbl

On Thu, Jun 17, 2021 at 05:18:21PM +0100, Cian Ferriter wrote:
> Signed-off-by: Cian Ferriter 
> 
> ---
> 
> v13:
> - New commit to update manpages with more commands that are missing.
> ---
>  lib/dpif-netdev-unixctl.man | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/lib/dpif-netdev-unixctl.man b/lib/dpif-netdev-unixctl.man
> index 45a1bd669..d77f5d9a4 100644
> --- a/lib/dpif-netdev-unixctl.man
> +++ b/lib/dpif-netdev-unixctl.man
> @@ -228,6 +228,16 @@ When this is the case, the above command prints the 
> load-balancing information
>  of the bonds configured in datapath \fIdp\fR showing the interface associated
>  with each bucket (hash).
>  .
> +.IP "\fBdpif-netdev/subtable-lookup-prio-get\fR"
> +Lists the DPCLS implementations or lookup functions that are available as 
> well
> +as their priorities.
> +.
> +.IP "\fBdpif-netdev/subtable-lookup-prio-set\fR \fIlookup_function\fR \
> +\fIprio\fR"
> +Sets the priority of a lookup function by the name, \fIlookup_function\fR, 
> and
> +the priority, \fIprio\fR, which should be a positive integer value. The 
> highest
> +priority lookup function is used for classification.
> +.
>  .IP "\fBdpif-netdev/dpif-get\fR
>  Lists the DPIF implementations that are available.
>  .
> -- 
> 2.32.0
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- 
fbl
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev