Re: [dpdk-dev] [PATCH] acl: remove invalid test
On Wed, Jan 18, 2017 at 09:51:45AM +, Ananyev, Konstantin wrote: > Hi Thomas, > > > > > 2016-12-23 09:36, Ananyev, Konstantin: > > > From: Michal Miroslaw [mailto:mirq-li...@rere.qmqm.pl] > > > > On Mon, Dec 19, 2016 at 06:48:52PM +, Ananyev, Konstantin wrote: > > > > > I suppose that changes have to be inside: > > > > > [PATCH v2] acl: allow zero verdict. > > > > > > > > The 'allow zero verdict' patch depends on this one if we are to not have > > > > a breaking tests inbetween. > > > > > > Exactly, that's why I think they either has to be in one series of > > > patches, > > > with this one coming first and ' PATCH v2] acl: allow zero verdict' as > > > the second one, > > > or just merge them into one. > > No progress here. > > Konstantin, do you ack this patch? > Yes, I do. > I just thought that the author would resubmit it as part of > ' PATCH v2] acl: allow zero verdict' > to comply with DPDK patch submission rules. > Konstantin > > I could apply it as a standalone patch. Sorry for the delay. I'll do just that in a moment. Best Regards, Michał Mirosław
Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
On Tue, Dec 13, 2016 at 10:36:16AM +, Ananyev, Konstantin wrote: > Hi Michal, > > > -Original Message- > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Michal Miroslaw > > Sent: Tuesday, December 13, 2016 1:08 AM > > To: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict > > > > Signed-off-by: Michał Mirosław > > --- > > lib/librte_acl/rte_acl.c | 3 +-- > > lib/librte_acl/rte_acl.h | 2 -- > > lib/librte_table/rte_table_acl.c | 2 +- > > 3 files changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c > > index 8b7e92c..d1f40be 100644 > > --- a/lib/librte_acl/rte_acl.c > > +++ b/lib/librte_acl/rte_acl.c > > @@ -313,8 +313,7 @@ acl_check_rule(const struct rte_acl_rule_data *rd) > > if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, typeof(rd->category_mask)) & > > rd->category_mask) == 0 || > > rd->priority > RTE_ACL_MAX_PRIORITY || > > - rd->priority < RTE_ACL_MIN_PRIORITY || > > - rd->userdata == RTE_ACL_INVALID_USERDATA) > > + rd->priority < RTE_ACL_MIN_PRIORITY) > > return -EINVAL; > > return 0; > > } > > I am not sure, how it supposed to work properly? > Zero value is reserved and ifnicates that no match were found for that input. This is actually in use by us. In our use we don't need to differentiate matching a rule with zero verdict vs not matching a rule at all. I also have a patch that changes the value returned in non-matching case, but it's in "dirty hack" state, as of yet. The ACL code does not treat zero userdata specially, so this is only a policy choice and as such would be better to be made by the user. Best Regards, Michał Mirosław
Re: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
On Tue, Dec 13, 2016 at 10:48:32AM +, Ananyev, Konstantin wrote:
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Michal Miroslaw
> > Sent: Tuesday, December 13, 2016 1:08 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
> >
> > From: Paweł Małachowski
> >
> > Thanks to that change we can use Null PMD for testing purposes.
> >
> > Signed-off-by: Michał Mirosław
> > ---
> > drivers/net/null/rte_eth_null.c | 13 -
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/null/rte_eth_null.c
> > b/drivers/net/null/rte_eth_null.c
> > index 836d982..f32ba2a 100644
> > --- a/drivers/net/null/rte_eth_null.c
> > +++ b/drivers/net/null/rte_eth_null.c
> > @@ -284,6 +284,9 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
> > tx_queue_id,
> > return 0;
> > }
> >
> > +static void
> > +eth_dev_void_ok(struct rte_eth_dev *dev __rte_unused) { return; }
> > +
> >
> > static void
> > eth_dev_info(struct rte_eth_dev *dev,
> > @@ -304,6 +307,9 @@ eth_dev_info(struct rte_eth_dev *dev,
> > dev_info->pci_dev = NULL;
> > dev_info->reta_size = internals->reta_size;
> > dev_info->flow_type_rss_offloads = internals->flow_type_rss_offloads;
> > + /* We hereby declare we can RX-offload VLAN-s out of thin air and
> > update checksums and VLANs before sinking packets in
> > /dev/null */
> > + dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
> > + dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT |
> > DEV_TX_OFFLOAD_IPV4_CKSUM;
>
> Hmm, how could it be supported if all that null PMD does on TX - just free
> the packets?
> Same question for RX.
You could imagine, that before dropping the packet you insert the tag
and calculate the checksum. The observed effect will be the same.
On RX this always indicates lack of VLAN tag. So whether the offload
is enabled or not it doesn't matter.
Best Regards,
Michał Mirosław
Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
On Tue, Dec 13, 2016 at 02:14:19PM +, Ananyev, Konstantin wrote: > > -Original Message- > > From: Michal Miroslaw [mailto:mirq-li...@rere.qmqm.pl] > > Sent: Tuesday, December 13, 2016 1:55 PM > > To: Ananyev, Konstantin > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict > > > > On Tue, Dec 13, 2016 at 10:36:16AM +, Ananyev, Konstantin wrote: > > > Hi Michal, > > > > > > > -Original Message- > > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Michal Miroslaw > > > > Sent: Tuesday, December 13, 2016 1:08 AM > > > > To: dev@dpdk.org > > > > Subject: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict > > > > > > > > Signed-off-by: Michał Mirosław > > > > --- > > > > lib/librte_acl/rte_acl.c | 3 +-- > > > > lib/librte_acl/rte_acl.h | 2 -- > > > > lib/librte_table/rte_table_acl.c | 2 +- > > > > 3 files changed, 2 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c > > > > index 8b7e92c..d1f40be 100644 > > > > --- a/lib/librte_acl/rte_acl.c > > > > +++ b/lib/librte_acl/rte_acl.c > > > > @@ -313,8 +313,7 @@ acl_check_rule(const struct rte_acl_rule_data *rd) > > > > if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, > > > > typeof(rd->category_mask)) & > > > > rd->category_mask) == 0 || > > > > rd->priority > RTE_ACL_MAX_PRIORITY || > > > > - rd->priority < RTE_ACL_MIN_PRIORITY || > > > > - rd->userdata == RTE_ACL_INVALID_USERDATA) > > > > + rd->priority < RTE_ACL_MIN_PRIORITY) > > > > return -EINVAL; > > > > return 0; > > > > } > > > > > > I am not sure, how it supposed to work properly? > > > Zero value is reserved and ifnicates that no match were found for that > > > input. > > > > This is actually in use by us. In our use we don't need to differentiate > > matching a rule with zero verdict vs not matching a rule at all. I also > > have a patch that changes the value returned in non-matching case, but > > it's in "dirty hack" state, as of yet. > > With that chane rte_acl_classify() might produce invalid results. > Even if you don't need it (I still don't understand how) , it doesn't mean > other people > don't need it either and it is ok to change it. > > > > > The ACL code does not treat zero userdata specially, so this is only > > a policy choice and as such would be better to be made by the user. > > I believe it does. > userdata==0 is a reserved value. > When rte_acl_clasify() returns 0 for that particular input, it means 'no > matches were found'. Dear Konstantin, Can you describe how the ACL code treats zero specially? I could not find anything, really. The only thing I found is that iff I use zero userdata in a rule I won't be able to differentiate a case where it matched from a case where no rule matched. If I all my rules have non-zero userdata, then this patch changes nothing. But if I have a table where 0 means drop (default-drop policy) then being able to use zero userdata in DROP rules makes the ACLs just that more useful. Best Regards, Michał Mirosław
Re: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
On Tue, Dec 13, 2016 at 02:37:34PM +, Ananyev, Konstantin wrote:
>
>
> > -Original Message-
> > From: Michal Miroslaw [mailto:mirq-li...@rere.qmqm.pl]
> > Sent: Tuesday, December 13, 2016 2:27 PM
> > To: Ananyev, Konstantin
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
> >
> > On Tue, Dec 13, 2016 at 10:48:32AM +, Ananyev, Konstantin wrote:
> > > > -Original Message-
> > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Michal Miroslaw
> > > > Sent: Tuesday, December 13, 2016 1:08 AM
> > > > To: dev@dpdk.org
> > > > Subject: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
> > > >
> > > > From: Paweł Małachowski
> > > >
> > > > Thanks to that change we can use Null PMD for testing purposes.
> > > >
> > > > Signed-off-by: Michał Mirosław
> > > > ---
> > > > drivers/net/null/rte_eth_null.c | 13 -
> > > > 1 file changed, 12 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/null/rte_eth_null.c
> > > > b/drivers/net/null/rte_eth_null.c
> > > > index 836d982..f32ba2a 100644
> > > > --- a/drivers/net/null/rte_eth_null.c
> > > > +++ b/drivers/net/null/rte_eth_null.c
> > > > @@ -284,6 +284,9 @@ eth_tx_queue_setup(struct rte_eth_dev *dev,
> > > > uint16_t tx_queue_id,
> > > > return 0;
> > > > }
> > > >
> > > > +static void
> > > > +eth_dev_void_ok(struct rte_eth_dev *dev __rte_unused) { return; }
> > > > +
> > > >
> > > > static void
> > > > eth_dev_info(struct rte_eth_dev *dev,
> > > > @@ -304,6 +307,9 @@ eth_dev_info(struct rte_eth_dev *dev,
> > > > dev_info->pci_dev = NULL;
> > > > dev_info->reta_size = internals->reta_size;
> > > > dev_info->flow_type_rss_offloads =
> > > > internals->flow_type_rss_offloads;
> > > > + /* We hereby declare we can RX-offload VLAN-s out of thin air
> > > > and update checksums and VLANs before sinking packets in
> > > > /dev/null */
> > > > + dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
> > > > + dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT |
> > > > DEV_TX_OFFLOAD_IPV4_CKSUM;
> > >
> > > Hmm, how could it be supported if all that null PMD does on TX - just
> > > free the packets?
> > > Same question for RX.
> >
> > You could imagine, that before dropping the packet you insert the tag
> > and calculate the checksum. The observed effect will be the same.
> >
> > On RX this always indicates lack of VLAN tag. So whether the offload
> > is enabled or not it doesn't matter.
>
> Of course user can imagine whatever he likes, but what the point to advertise
> these capabilities,
> when the PMD clearly doesn't have them?
> Again, why these particular ones?
Hmm. I guess we can expand the set. Those were the ones we actually use
(depend on) for testing our code.
This allows to use null PMD for testing in place of real network interface
with those offloads. This patch is to keep users from having to place if's
around their code just to support test scenarios with null PMD.
Best Regards,
Michał Mirosław
Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
On Tue, Dec 13, 2016 at 03:13:42PM +, Ananyev, Konstantin wrote: [...] > > > > > > Subject: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict > > > > > > > > > > > > Signed-off-by: Michał Mirosław > > > > > > --- > > > > > > lib/librte_acl/rte_acl.c | 3 +-- > > > > > > lib/librte_acl/rte_acl.h | 2 -- > > > > > > lib/librte_table/rte_table_acl.c | 2 +- > > > > > > 3 files changed, 2 insertions(+), 5 deletions(-) > > > > > > > > > > > > diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c > > > > > > index 8b7e92c..d1f40be 100644 > > > > > > --- a/lib/librte_acl/rte_acl.c > > > > > > +++ b/lib/librte_acl/rte_acl.c > > > > > > @@ -313,8 +313,7 @@ acl_check_rule(const struct rte_acl_rule_data > > > > > > *rd) > > > > > > if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, > > > > > > typeof(rd->category_mask)) & > > > > > > rd->category_mask) == 0 || > > > > > > rd->priority > RTE_ACL_MAX_PRIORITY || > > > > > > - rd->priority < RTE_ACL_MIN_PRIORITY || > > > > > > - rd->userdata == RTE_ACL_INVALID_USERDATA) > > > > > > + rd->priority < RTE_ACL_MIN_PRIORITY) > > > > > > return -EINVAL; > > > > > > return 0; > > > > > > } > > > > > > > > > > I am not sure, how it supposed to work properly? > > > > > Zero value is reserved and ifnicates that no match were found for > > > > > that input. > > > > > > > > This is actually in use by us. In our use we don't need to differentiate > > > > matching a rule with zero verdict vs not matching a rule at all. I also > > > > have a patch that changes the value returned in non-matching case, but > > > > it's in "dirty hack" state, as of yet. > > > > > > With that chane rte_acl_classify() might produce invalid results. > > > Even if you don't need it (I still don't understand how) , it doesn't > > > mean other people > > > don't need it either and it is ok to change it. > > > > > > > > > > > The ACL code does not treat zero userdata specially, so this is only > > > > a policy choice and as such would be better to be made by the user. > > > > > > I believe it does. > > > userdata==0 is a reserved value. > > > When rte_acl_clasify() returns 0 for that particular input, it means 'no > > > matches were found'. > > > > Dear Konstantin, > > > > Can you describe how the ACL code treats zero specially? I could not find > > anything, really. The only thing I found is that iff I use zero userdata > > in a rule I won't be able to differentiate a case where it matched from > > a case where no rule matched. > > Yes, that's what I am talking about. > > > If I all my rules have non-zero userdata, > > then this patch changes nothing. > > Ok, then why do you remove a code that does checking for invalid userdata==0? > That supposed to prevent user to setup invalid value by mistake. > > But if I have a table where 0 means drop > > (default-drop policy) then being able to use zero userdata in DROP rules > > makes the ACLs just that more useful. > > Ok, and what prevents you from do +1 to your policy values before > you insert it into the ACL table and -1 after you retrieved it via > rte_acl_classify()? The check is enforcing an assumption that all users want to distinguish the cases whether any rule matched and whether no rules matched. Not all users do, hence the assumption is invalid and this patch removes it. Yes, people can work around it by loosing 1 of 2^32 useful values and convoluting their code. You seem to argue that 0 is somehow an invalid value, but I can't find anything in the ACL that would require it to be so. Could you point me to the code in DPDK where this actually matters? Best Regards, Michał Mirosław
Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
On Tue, Dec 13, 2016 at 05:14:09PM +0100, Michal Miroslaw wrote: > On Tue, Dec 13, 2016 at 03:13:42PM +, Ananyev, Konstantin wrote: [...] > > > Dear Konstantin, > > > > > > Can you describe how the ACL code treats zero specially? I could not find > > > anything, really. The only thing I found is that iff I use zero userdata > > > in a rule I won't be able to differentiate a case where it matched from > > > a case where no rule matched. > > > > Yes, that's what I am talking about. > > > > > If I all my rules have non-zero userdata, > > > then this patch changes nothing. > > > > Ok, then why do you remove a code that does checking for invalid > > userdata==0? > > That supposed to prevent user to setup invalid value by mistake. > > > > But if I have a table where 0 means drop > > > (default-drop policy) then being able to use zero userdata in DROP rules > > > makes the ACLs just that more useful. > > > > Ok, and what prevents you from do +1 to your policy values before > > you insert it into the ACL table and -1 after you retrieved it via > > rte_acl_classify()? > > The check is enforcing an assumption that all users want to distinguish > the cases whether any rule matched and whether no rules matched. Not all > users do, hence the assumption is invalid and this patch removes it. > > Yes, people can work around it by loosing 1 of 2^32 useful values and > convoluting their code. > > You seem to argue that 0 is somehow an invalid value, but I can't find > anything in the ACL that would require it to be so. Could you point me > to the code in DPDK where this actually matters? I just noticed that it's probably you who wrote most of the ACLs code, so I guest you're the right person to ask the question above. Nice work, BTW. :-) Best Regards, Michał Mirosław
Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
On Tue, Dec 13, 2016 at 05:27:31PM +, Ananyev, Konstantin wrote: > > > > -Original Message- > > From: Michal Miroslaw [mailto:mirq-li...@rere.qmqm.pl] > > Sent: Tuesday, December 13, 2016 4:14 PM > > To: Ananyev, Konstantin > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict > > > > On Tue, Dec 13, 2016 at 03:13:42PM +, Ananyev, Konstantin wrote: > > [...] > > > > > > > > Subject: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict > > > > > > > > > > > > > > > > Signed-off-by: Michał Mirosław > > > > > > > > > > > > > > > > --- > > > > > > > > lib/librte_acl/rte_acl.c | 3 +-- > > > > > > > > lib/librte_acl/rte_acl.h | 2 -- > > > > > > > > lib/librte_table/rte_table_acl.c | 2 +- > > > > > > > > 3 files changed, 2 insertions(+), 5 deletions(-) > > > > > > > > > > > > > > > > diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c > > > > > > > > index 8b7e92c..d1f40be 100644 > > > > > > > > --- a/lib/librte_acl/rte_acl.c > > > > > > > > +++ b/lib/librte_acl/rte_acl.c > > > > > > > > @@ -313,8 +313,7 @@ acl_check_rule(const struct > > > > > > > > rte_acl_rule_data *rd) > > > > > > > > if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, > > > > > > > > typeof(rd->category_mask)) & > > > > > > > > rd->category_mask) == 0 || > > > > > > > > rd->priority > RTE_ACL_MAX_PRIORITY || > > > > > > > > - rd->priority < RTE_ACL_MIN_PRIORITY || > > > > > > > > - rd->userdata == > > > > > > > > RTE_ACL_INVALID_USERDATA) > > > > > > > > + rd->priority < RTE_ACL_MIN_PRIORITY) > > > > > > > > return -EINVAL; > > > > > > > > return 0; > > > > > > > > } > > > > > > > > > > > > > > I am not sure, how it supposed to work properly? > > > > > > > Zero value is reserved and ifnicates that no match were found for > > > > > > > that input. > > > > > > > > > > > > This is actually in use by us. In our use we don't need to > > > > > > differentiate > > > > > > matching a rule with zero verdict vs not matching a rule at all. I > > > > > > also > > > > > > have a patch that changes the value returned in non-matching case, > > > > > > but > > > > > > it's in "dirty hack" state, as of yet. > > > > > > > > > > With that chane rte_acl_classify() might produce invalid results. > > > > > Even if you don't need it (I still don't understand how) , it doesn't > > > > > mean other people > > > > > don't need it either and it is ok to change it. > > > > > > > > > > > > > > > > > The ACL code does not treat zero userdata specially, so this is only > > > > > > a policy choice and as such would be better to be made by the user. > > > > > > > > > > I believe it does. > > > > > userdata==0 is a reserved value. > > > > > When rte_acl_clasify() returns 0 for that particular input, it means > > > > > 'no matches were found'. > > > > > > > > Dear Konstantin, > > > > > > > > Can you describe how the ACL code treats zero specially? I could not > > > > find > > > > anything, really. The only thing I found is that iff I use zero userdata > > > > in a rule I won't be able to differentiate a case where it matched from > > > > a case where no rule matched. > > > > > > Yes, that's what I am talking about. > > > > > > > If I all my rules have non-zero userdata, > > > > then this patch changes nothing. > > > > > > Ok, then why do you remove a code that does checking for invalid > > > userdata==0? > > > That supposed to prevent user to s
Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
On Tue, Dec 13, 2016 at 09:55:12PM +, Ananyev, Konstantin wrote: > > > > -Original Message- > > From: Michal Miroslaw [mailto:mirq-li...@rere.qmqm.pl] > > Sent: Tuesday, December 13, 2016 6:03 PM > > To: Ananyev, Konstantin > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict > > > > On Tue, Dec 13, 2016 at 05:27:31PM +, Ananyev, Konstantin wrote: > > > > > > > > > > -Original Message- > > > > From: Michal Miroslaw [mailto:mirq-li...@rere.qmqm.pl] > > > > Sent: Tuesday, December 13, 2016 4:14 PM > > > > To: Ananyev, Konstantin > > > > Cc: dev@dpdk.org > > > > Subject: Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict > > > > > > > > On Tue, Dec 13, 2016 at 03:13:42PM +, Ananyev, Konstantin wrote: > > > > [...] > > > > > > > > > > Subject: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict > > > > > > > > > > > > > > > > > > > > Signed-off-by: Michał Mirosław > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > lib/librte_acl/rte_acl.c | 3 +-- > > > > > > > > > > lib/librte_acl/rte_acl.h | 2 -- > > > > > > > > > > lib/librte_table/rte_table_acl.c | 2 +- > > > > > > > > > > 3 files changed, 2 insertions(+), 5 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/lib/librte_acl/rte_acl.c > > > > > > > > > > b/lib/librte_acl/rte_acl.c > > > > > > > > > > index 8b7e92c..d1f40be 100644 > > > > > > > > > > --- a/lib/librte_acl/rte_acl.c > > > > > > > > > > +++ b/lib/librte_acl/rte_acl.c > > > > > > > > > > @@ -313,8 +313,7 @@ acl_check_rule(const struct > > > > > > > > > > rte_acl_rule_data *rd) > > > > > > > > > > if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, > > > > > > > > > > typeof(rd->category_mask)) & > > > > > > > > > > rd->category_mask) == 0 || > > > > > > > > > > rd->priority > RTE_ACL_MAX_PRIORITY || > > > > > > > > > > - rd->priority < RTE_ACL_MIN_PRIORITY || > > > > > > > > > > - rd->userdata == > > > > > > > > > > RTE_ACL_INVALID_USERDATA) > > > > > > > > > > + rd->priority < RTE_ACL_MIN_PRIORITY) > > > > > > > > > > return -EINVAL; > > > > > > > > > > return 0; > > > > > > > > > > } > > > > > > > > > > > > > > > > > > I am not sure, how it supposed to work properly? > > > > > > > > > Zero value is reserved and ifnicates that no match were found > > > > > > > > > for that input. > > > > > > > > > > > > > > > > This is actually in use by us. In our use we don't need to > > > > > > > > differentiate > > > > > > > > matching a rule with zero verdict vs not matching a rule at > > > > > > > > all. I also > > > > > > > > have a patch that changes the value returned in non-matching > > > > > > > > case, but > > > > > > > > it's in "dirty hack" state, as of yet. > > > > > > > > > > > > > > With that chane rte_acl_classify() might produce invalid results. > > > > > > > Even if you don't need it (I still don't understand how) , it > > > > > > > doesn't mean other people > > > > > > > don't need it either and it is ok to change it. > > > > > > > > > > > > > > > > > > > > > > > The ACL code does not treat zero userdata specially, so this is > > > > > > > > only > > > > > > > > a policy choice and as such would be better to be made by the > > > > > > > > user. > &
Re: [dpdk-dev] [PATCH] acl: remove invalid test
On Mon, Dec 19, 2016 at 06:48:52PM +, Ananyev, Konstantin wrote: > Hi Michal, > > > -Original Message- > > From: Michał Mirosław [mailto:mirq-li...@rere.qmqm.pl] > > Sent: Wednesday, December 14, 2016 5:24 PM > > To: dev@dpdk.org > > Cc: Ananyev, Konstantin > > Subject: [PATCH] acl: remove invalid test > > > > rte_acl_add_rules() has no way of checking rule size. > > > > This was hidden because the test effectively checked that > > adding a rule with userdata == 0 failed. > I suppose that changes have to be inside: > [PATCH v2] acl: allow zero verdict. The 'allow zero verdict' patch depends on this one if we are to not have a breaking tests inbetween. Otherwise, it is an independent change. I guess I can merge them, though, if you prefer it that way. Best Regards, Michał Mirosław
