Re: [vpp-dev] Bind / Unbind of ACL

2017-06-19 Thread Marco Varlese
On Sat, 2017-06-17 at 11:27 +0200, Andrew   Yourtchenko wrote:
> Perfect, thanks a lot!
> 
> I've pushed the update https://gerrit.fd.io/r/#/c/6858/10 which
> codifies all of our discussion.
> 
> With that change, both applying an inexistent ACL and deleting the ACL
> that is applied somewhere will be an error.
> 
> One can still create an ACL with no rules, in which case applying that
> ACL will follow the lookup logic with the default-deny in the end of
> the vector of ACLs lookup.
+1

> 
> --a
> 
> On 6/17/17, Luke, Chris <chris_l...@comcast.com> wrote:
> > 
> > That was going to be one of my queries, but forgot to ask. Do we allow that
> > currently? Probably shouldn't, as you say, for symmetry.
> > 
> > Chris.
> > 
> > > 
> > > -Original Message-
> > > From: Andrew Yourtchenko [mailto:ayour...@gmail.com]
> > > Sent: Friday, June 16, 2017 17:51
> > > To: Luke, Chris <chris_l...@cable.comcast.com>
> > > Cc: Marco Varlese <marco.varl...@suse.com>; vpp-dev@lists.fd.io
> > > Subject: Re: [vpp-dev] Bind / Unbind of ACL
> > > 
> > > Ok! So what do you think if then we were to also disallow applying the
> > > ACL
> > > that doesn't exist yet ?
> > > 
> > > It feels like it would be a matching symmetric behavior "from the other
> > > side".
> > > ?
> > > 
> > > --a
> > > 
> > > On 16 Jun 2017, at 15:38, Luke, Chris <chris_l...@comcast.com> wrote:
> > > 
> > > > 
> > > > > 
> > > > > From: Marco Varlese [mailto:marco.varl...@suse.com]
> > > > > Sent: Friday, June 16, 2017 9:23
> > > > > > 
> > > > > > On Fri, 2017-06-16 at 15:12 +0200, Andrew   Yourtchenko wrote:
> > > > > > > 
> > > > > > > On 6/16/17, Marco Varlese <marco.varl...@suse.com> wrote:
> > > > > > > 
> > > > > > > > 
> > > > > > > > On Thu, 2017-06-15 at 14:22 +0200, Andrew   Yourtchenko wrote:
> > > > > > > > 
> > > > > > > > After a bit more thinking - there is a way that should take care
> > > > > > > > of
> > > > > > > > both:
> > > > > > > > 
> > > > > > > > 1) What Chris wrote: have consistent behaviour with non-existent
> > > > > > > > ACL as if the ACL matching fell off the end of the ACL: if an
> > > > > > > > empty ACL is referenced, treat it as if it had entries but none
> > > > > > > > of
> > > > > > > > them had matched. Then we still hit the "default deny" if none
> > > > > > > > of
> > > > > > > > the applied ACLs match, and drop the packets - so it will be
> > > > > > > > logical.
> > > > > > > > 
> > > > > > > > 2) What Marco wrote: when deleting an already referenced ACL,
> > > > > > > > unapply it from all the places where it is applied.
> > > > > > > > 
> > > > > > > > It's a change in the behaviour in that the current behaviour is
> > > > > > > > to
> > > > > > > > have the empty ACL act as if it was "deny any any", and block
> > > > > > > > the
> > > > > > > > matching even if there is another ACL after it - but on the
> > > > > > > > other
> > > > > > > > hand this would take both viewpoints in mind...
> > > > > > > I think this approach would still leave the system in an
> > > > > > > inconsistent
> > > state:
> > > > 
> > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > an
> > > > > > > interface is basically assigned an ACL which does not exist in the
> > > system.
> > > > 
> > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > Also, the risk I see is if I later on create an ACL with the
> > > > > > > previously used index then an interface would see that ACL being
> > > > > > > applied automatically (since it's referenced). However, I may not
> > > > > > > wan

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-17 Thread Luke, Chris
+1

> -Original Message-
> From: Andrew  Yourtchenko [mailto:ayour...@gmail.com]
> Sent: Saturday, June 17, 2017 5:28
> To: Luke, Chris <chris_l...@cable.comcast.com>
> Cc: Marco Varlese <marco.varl...@suse.com>; vpp-dev@lists.fd.io
> Subject: Re: [vpp-dev] Bind / Unbind of ACL
> 
> Perfect, thanks a lot!
> 
> I've pushed the update https://gerrit.fd.io/r/#/c/6858/10 which codifies all 
> of
> our discussion.
> 
> With that change, both applying an inexistent ACL and deleting the ACL that
> is applied somewhere will be an error.
> 
> One can still create an ACL with no rules, in which case applying that ACL 
> will
> follow the lookup logic with the default-deny in the end of the vector of ACLs
> lookup.
> 
> --a
> 
> On 6/17/17, Luke, Chris <chris_l...@comcast.com> wrote:
> > That was going to be one of my queries, but forgot to ask. Do we allow
> > that currently? Probably shouldn't, as you say, for symmetry.
> >
> > Chris.
> >
> >> -Original Message-
> >> From: Andrew Yourtchenko [mailto:ayour...@gmail.com]
> >> Sent: Friday, June 16, 2017 17:51
> >> To: Luke, Chris <chris_l...@cable.comcast.com>
> >> Cc: Marco Varlese <marco.varl...@suse.com>; vpp-dev@lists.fd.io
> >> Subject: Re: [vpp-dev] Bind / Unbind of ACL
> >>
> >> Ok! So what do you think if then we were to also disallow applying
> >> the ACL that doesn't exist yet ?
> >>
> >> It feels like it would be a matching symmetric behavior "from the
> >> other side".
> >> ?
> >>
> >> --a
> >>
> >> On 16 Jun 2017, at 15:38, Luke, Chris <chris_l...@comcast.com> wrote:
> >>
> >> >> From: Marco Varlese [mailto:marco.varl...@suse.com]
> >> >> Sent: Friday, June 16, 2017 9:23
> >> >>> On Fri, 2017-06-16 at 15:12 +0200, Andrew   Yourtchenko wrote:
> >> >>>> On 6/16/17, Marco Varlese <marco.varl...@suse.com> wrote:
> >> >>>>
> >> >>>>> On Thu, 2017-06-15 at 14:22 +0200, Andrew   Yourtchenko wrote:
> >> >>>>>
> >> >>>>> After a bit more thinking - there is a way that should take
> >> >>>>> care of
> >> >>>>> both:
> >> >>>>>
> >> >>>>> 1) What Chris wrote: have consistent behaviour with
> >> >>>>> non-existent ACL as if the ACL matching fell off the end of the
> >> >>>>> ACL: if an empty ACL is referenced, treat it as if it had
> >> >>>>> entries but none of them had matched. Then we still hit the
> >> >>>>> "default deny" if none of the applied ACLs match, and drop the
> >> >>>>> packets - so it will be logical.
> >> >>>>>
> >> >>>>> 2) What Marco wrote: when deleting an already referenced ACL,
> >> >>>>> unapply it from all the places where it is applied.
> >> >>>>>
> >> >>>>> It's a change in the behaviour in that the current behaviour is
> >> >>>>> to have the empty ACL act as if it was "deny any any", and
> >> >>>>> block the matching even if there is another ACL after it - but
> >> >>>>> on the other hand this would take both viewpoints in mind...
> >> >>>> I think this approach would still leave the system in an
> >> >>>> inconsistent
> >> state:
> >> >>>> an
> >> >>>> interface is basically assigned an ACL which does not exist in
> >> >>>> the
> >> system.
> >> >>>> Also, the risk I see is if I later on create an ACL with the
> >> >>>> previously used index then an interface would see that ACL being
> >> >>>> applied automatically (since it's referenced). However, I may
> >> >>>> not want to have that ACL assigned to that particular interface.
> >> >>>> Correct?
> >> >>>>
> >> >>>> I think that the "deletion" of an ACL could see one of the two
> >> >>>> behaviours
> >> >>>> below:
> >> >>>> 1) the deletion of an ACL should be DENIED if that ACL is
> >> >>>> assigned to any interface (probably the easier and safer
> >> >>>> approach);
>

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-17 Thread Andrew  Yourtchenko
Perfect, thanks a lot!

I've pushed the update https://gerrit.fd.io/r/#/c/6858/10 which
codifies all of our discussion.

With that change, both applying an inexistent ACL and deleting the ACL
that is applied somewhere will be an error.

One can still create an ACL with no rules, in which case applying that
ACL will follow the lookup logic with the default-deny in the end of
the vector of ACLs lookup.

--a

On 6/17/17, Luke, Chris <chris_l...@comcast.com> wrote:
> That was going to be one of my queries, but forgot to ask. Do we allow that
> currently? Probably shouldn't, as you say, for symmetry.
>
> Chris.
>
>> -Original Message-
>> From: Andrew Yourtchenko [mailto:ayour...@gmail.com]
>> Sent: Friday, June 16, 2017 17:51
>> To: Luke, Chris <chris_l...@cable.comcast.com>
>> Cc: Marco Varlese <marco.varl...@suse.com>; vpp-dev@lists.fd.io
>> Subject: Re: [vpp-dev] Bind / Unbind of ACL
>>
>> Ok! So what do you think if then we were to also disallow applying the
>> ACL
>> that doesn't exist yet ?
>>
>> It feels like it would be a matching symmetric behavior "from the other
>> side".
>> ?
>>
>> --a
>>
>> On 16 Jun 2017, at 15:38, Luke, Chris <chris_l...@comcast.com> wrote:
>>
>> >> From: Marco Varlese [mailto:marco.varl...@suse.com]
>> >> Sent: Friday, June 16, 2017 9:23
>> >>> On Fri, 2017-06-16 at 15:12 +0200, Andrew   Yourtchenko wrote:
>> >>>> On 6/16/17, Marco Varlese <marco.varl...@suse.com> wrote:
>> >>>>
>> >>>>> On Thu, 2017-06-15 at 14:22 +0200, Andrew   Yourtchenko wrote:
>> >>>>>
>> >>>>> After a bit more thinking - there is a way that should take care of
>> >>>>> both:
>> >>>>>
>> >>>>> 1) What Chris wrote: have consistent behaviour with non-existent
>> >>>>> ACL as if the ACL matching fell off the end of the ACL: if an
>> >>>>> empty ACL is referenced, treat it as if it had entries but none of
>> >>>>> them had matched. Then we still hit the "default deny" if none of
>> >>>>> the applied ACLs match, and drop the packets - so it will be
>> >>>>> logical.
>> >>>>>
>> >>>>> 2) What Marco wrote: when deleting an already referenced ACL,
>> >>>>> unapply it from all the places where it is applied.
>> >>>>>
>> >>>>> It's a change in the behaviour in that the current behaviour is to
>> >>>>> have the empty ACL act as if it was "deny any any", and block the
>> >>>>> matching even if there is another ACL after it - but on the other
>> >>>>> hand this would take both viewpoints in mind...
>> >>>> I think this approach would still leave the system in an
>> >>>> inconsistent
>> state:
>> >>>> an
>> >>>> interface is basically assigned an ACL which does not exist in the
>> system.
>> >>>> Also, the risk I see is if I later on create an ACL with the
>> >>>> previously used index then an interface would see that ACL being
>> >>>> applied automatically (since it's referenced). However, I may not
>> >>>> want to have that ACL assigned to that particular interface.
>> >>>> Correct?
>> >>>>
>> >>>> I think that the "deletion" of an ACL could see one of the two
>> >>>> behaviours
>> >>>> below:
>> >>>> 1) the deletion of an ACL should be DENIED if that ACL is assigned
>> >>>> to any interface (probably the easier and safer approach);
>> >>>> 2) the deletion of an ACL should see a CASCADING effect onto the
>> >>>> interfaces which would be "cleaned up" of any references to that
>> >>>> ACL;
>> >>>>
>> >>>
>> >>> Right, the (2) was what I was trying to suggest to do...
>> >>>
>> >>>>
>> >>>> I think (1) is a very good way of solving the initial problem since
>> >>>> it works nicely if you manage VPP directly (e.g. via command-line)
>> >>>> and if you use a controller. In the latter, the controller can
>> >>>> react on the "error" returned by the acl_del API because that ACL
>> >>>> is assigned somewhere.
>> >>&g

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-16 Thread Luke, Chris
That was going to be one of my queries, but forgot to ask. Do we allow that 
currently? Probably shouldn't, as you say, for symmetry.

Chris.

> -Original Message-
> From: Andrew Yourtchenko [mailto:ayour...@gmail.com]
> Sent: Friday, June 16, 2017 17:51
> To: Luke, Chris <chris_l...@cable.comcast.com>
> Cc: Marco Varlese <marco.varl...@suse.com>; vpp-dev@lists.fd.io
> Subject: Re: [vpp-dev] Bind / Unbind of ACL
> 
> Ok! So what do you think if then we were to also disallow applying the ACL
> that doesn't exist yet ?
> 
> It feels like it would be a matching symmetric behavior "from the other side".
> ?
> 
> --a
> 
> On 16 Jun 2017, at 15:38, Luke, Chris <chris_l...@comcast.com> wrote:
> 
> >> From: Marco Varlese [mailto:marco.varl...@suse.com]
> >> Sent: Friday, June 16, 2017 9:23
> >>> On Fri, 2017-06-16 at 15:12 +0200, Andrew   Yourtchenko wrote:
> >>>> On 6/16/17, Marco Varlese <marco.varl...@suse.com> wrote:
> >>>>
> >>>>> On Thu, 2017-06-15 at 14:22 +0200, Andrew   Yourtchenko wrote:
> >>>>>
> >>>>> After a bit more thinking - there is a way that should take care of 
> >>>>> both:
> >>>>>
> >>>>> 1) What Chris wrote: have consistent behaviour with non-existent
> >>>>> ACL as if the ACL matching fell off the end of the ACL: if an
> >>>>> empty ACL is referenced, treat it as if it had entries but none of
> >>>>> them had matched. Then we still hit the "default deny" if none of
> >>>>> the applied ACLs match, and drop the packets - so it will be logical.
> >>>>>
> >>>>> 2) What Marco wrote: when deleting an already referenced ACL,
> >>>>> unapply it from all the places where it is applied.
> >>>>>
> >>>>> It's a change in the behaviour in that the current behaviour is to
> >>>>> have the empty ACL act as if it was "deny any any", and block the
> >>>>> matching even if there is another ACL after it - but on the other
> >>>>> hand this would take both viewpoints in mind...
> >>>> I think this approach would still leave the system in an inconsistent
> state:
> >>>> an
> >>>> interface is basically assigned an ACL which does not exist in the
> system.
> >>>> Also, the risk I see is if I later on create an ACL with the
> >>>> previously used index then an interface would see that ACL being
> >>>> applied automatically (since it's referenced). However, I may not
> >>>> want to have that ACL assigned to that particular interface.
> >>>> Correct?
> >>>>
> >>>> I think that the "deletion" of an ACL could see one of the two
> >>>> behaviours
> >>>> below:
> >>>> 1) the deletion of an ACL should be DENIED if that ACL is assigned
> >>>> to any interface (probably the easier and safer approach);
> >>>> 2) the deletion of an ACL should see a CASCADING effect onto the
> >>>> interfaces which would be "cleaned up" of any references to that
> >>>> ACL;
> >>>>
> >>>
> >>> Right, the (2) was what I was trying to suggest to do...
> >>>
> >>>>
> >>>> I think (1) is a very good way of solving the initial problem since
> >>>> it works nicely if you manage VPP directly (e.g. via command-line)
> >>>> and if you use a controller. In the latter, the controller can
> >>>> react on the "error" returned by the acl_del API because that ACL
> >>>> is assigned somewhere.
> >>>>
> >>>
> >>> ...but the (1) is another good option to me.
> >>>
> >>> So it seems we are converging on (1) ?
> >> I would go with (1)...
> >
> > I feel I have a slight preference for this (1) also; In general I don't 
> > like the
> implicit actions such as in (2).
> >
> > Chris
> >
> >>>
> >>> --a
> >>>
> >>>
> >>>
> >>>>
> >>>>
> >>>> Cheers,
> >>>> Marco
> >>>>
> >>>>>
> >>>>>
> >>>>> What do you think ?
> >>>>>
> >>>>> --a
> >>>>>
> >>>>>> On 6/9/17, Andrew   Your

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-16 Thread Andrew Yourtchenko
Ok! So what do you think if then we were to also disallow applying the ACL that 
doesn't exist yet ?

It feels like it would be a matching symmetric behavior "from the other side". ?

--a

On 16 Jun 2017, at 15:38, Luke, Chris <chris_l...@comcast.com> wrote:

>> From: Marco Varlese [mailto:marco.varl...@suse.com]
>> Sent: Friday, June 16, 2017 9:23
>>> On Fri, 2017-06-16 at 15:12 +0200, Andrew   Yourtchenko wrote:
>>>> On 6/16/17, Marco Varlese <marco.varl...@suse.com> wrote:
>>>> 
>>>>> On Thu, 2017-06-15 at 14:22 +0200, Andrew   Yourtchenko wrote:
>>>>> 
>>>>> After a bit more thinking - there is a way that should take care of both:
>>>>> 
>>>>> 1) What Chris wrote: have consistent behaviour with non-existent
>>>>> ACL as if the ACL matching fell off the end of the ACL: if an
>>>>> empty ACL is referenced, treat it as if it had entries but none of
>>>>> them had matched. Then we still hit the "default deny" if none of
>>>>> the applied ACLs match, and drop the packets - so it will be logical.
>>>>> 
>>>>> 2) What Marco wrote: when deleting an already referenced ACL,
>>>>> unapply it from all the places where it is applied.
>>>>> 
>>>>> It's a change in the behaviour in that the current behaviour is to
>>>>> have the empty ACL act as if it was "deny any any", and block the
>>>>> matching even if there is another ACL after it - but on the other
>>>>> hand this would take both viewpoints in mind...
>>>> I think this approach would still leave the system in an inconsistent 
>>>> state:
>>>> an
>>>> interface is basically assigned an ACL which does not exist in the system.
>>>> Also, the risk I see is if I later on create an ACL with the
>>>> previously used index then an interface would see that ACL being
>>>> applied automatically (since it's referenced). However, I may not
>>>> want to have that ACL assigned to that particular interface.
>>>> Correct?
>>>> 
>>>> I think that the "deletion" of an ACL could see one of the two
>>>> behaviours
>>>> below:
>>>> 1) the deletion of an ACL should be DENIED if that ACL is assigned
>>>> to any interface (probably the easier and safer approach);
>>>> 2) the deletion of an ACL should see a CASCADING effect onto the
>>>> interfaces which would be "cleaned up" of any references to that
>>>> ACL;
>>>> 
>>> 
>>> Right, the (2) was what I was trying to suggest to do...
>>> 
>>>> 
>>>> I think (1) is a very good way of solving the initial problem since
>>>> it works nicely if you manage VPP directly (e.g. via command-line)
>>>> and if you use a controller. In the latter, the controller can react
>>>> on the "error" returned by the acl_del API because that ACL is
>>>> assigned somewhere.
>>>> 
>>> 
>>> ...but the (1) is another good option to me.
>>> 
>>> So it seems we are converging on (1) ?
>> I would go with (1)...
> 
> I feel I have a slight preference for this (1) also; In general I don't like 
> the implicit actions such as in (2).
> 
> Chris
> 
>>> 
>>> --a
>>> 
>>> 
>>> 
>>>> 
>>>> 
>>>> Cheers,
>>>> Marco
>>>> 
>>>>> 
>>>>> 
>>>>> What do you think ?
>>>>> 
>>>>> --a
>>>>> 
>>>>>> On 6/9/17, Andrew   Yourtchenko <ayour...@gmail.com> wrote:
>>>>>> 
>>>>>> 
>>>>>> Assuming the only change is to effectively have
>>>>>> "unbind_acl_from_everywhere; delete_acl" instead of
>>>>>> "delete_acl", maybe it would be best to tackle that post-17.07
>>>>>> with a separate API message acl_del_and_unbind or similar ?
>>>>>> 
>>>>>> I feel a beet wary of adding more hidden state (even though the
>>>>>> reflected sessions table does provide already plenty of it :)
>>>>>> 
>>>>>> --a
>>>>>> 
>>>>>>> On 6/9/17, Luke, Chris <chris_l...@comcast.com> wrote:
>>>>>>> 
>>>>>>> 
>>>>>>> Would it 

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-16 Thread Luke, Chris
> From: Marco Varlese [mailto:marco.varl...@suse.com]
> Sent: Friday, June 16, 2017 9:23
> On Fri, 2017-06-16 at 15:12 +0200, Andrew   Yourtchenko wrote:
> > On 6/16/17, Marco Varlese <marco.varl...@suse.com> wrote:
> > >
> > > On Thu, 2017-06-15 at 14:22 +0200, Andrew   Yourtchenko wrote:
> > > >
> > > > After a bit more thinking - there is a way that should take care of 
> > > > both:
> > > >
> > > > 1) What Chris wrote: have consistent behaviour with non-existent
> > > > ACL as if the ACL matching fell off the end of the ACL: if an
> > > > empty ACL is referenced, treat it as if it had entries but none of
> > > > them had matched. Then we still hit the "default deny" if none of
> > > > the applied ACLs match, and drop the packets - so it will be logical.
> > > >
> > > > 2) What Marco wrote: when deleting an already referenced ACL,
> > > > unapply it from all the places where it is applied.
> > > >
> > > > It's a change in the behaviour in that the current behaviour is to
> > > > have the empty ACL act as if it was "deny any any", and block the
> > > > matching even if there is another ACL after it - but on the other
> > > > hand this would take both viewpoints in mind...
> > > I think this approach would still leave the system in an inconsistent 
> > > state:
> > > an
> > > interface is basically assigned an ACL which does not exist in the system.
> > > Also, the risk I see is if I later on create an ACL with the
> > > previously used index then an interface would see that ACL being
> > > applied automatically (since it's referenced). However, I may not
> > > want to have that ACL assigned to that particular interface.
> > > Correct?
> > >
> > > I think that the "deletion" of an ACL could see one of the two
> > > behaviours
> > > below:
> > > 1) the deletion of an ACL should be DENIED if that ACL is assigned
> > > to any interface (probably the easier and safer approach);
> > > 2) the deletion of an ACL should see a CASCADING effect onto the
> > > interfaces which would be "cleaned up" of any references to that
> > > ACL;
> > >
> >
> > Right, the (2) was what I was trying to suggest to do...
> >
> > >
> > > I think (1) is a very good way of solving the initial problem since
> > > it works nicely if you manage VPP directly (e.g. via command-line)
> > > and if you use a controller. In the latter, the controller can react
> > > on the "error" returned by the acl_del API because that ACL is
> > > assigned somewhere.
> > >
> >
> > ...but the (1) is another good option to me.
> >
> > So it seems we are converging on (1) ?
> I would go with (1)...

I feel I have a slight preference for this (1) also; In general I don't like 
the implicit actions such as in (2).

Chris

> >
> > --a
> >
> >
> >
> > >
> > >
> > > Cheers,
> > > Marco
> > >
> > > >
> > > >
> > > > What do you think ?
> > > >
> > > > --a
> > > >
> > > > On 6/9/17, Andrew   Yourtchenko <ayour...@gmail.com> wrote:
> > > > >
> > > > >
> > > > > Assuming the only change is to effectively have
> > > > > "unbind_acl_from_everywhere; delete_acl" instead of
> > > > > "delete_acl", maybe it would be best to tackle that post-17.07
> > > > > with a separate API message acl_del_and_unbind or similar ?
> > > > >
> > > > > I feel a beet wary of adding more hidden state (even though the
> > > > > reflected sessions table does provide already plenty of it :)
> > > > >
> > > > > --a
> > > > >
> > > > > On 6/9/17, Luke, Chris <chris_l...@comcast.com> wrote:
> > > > > >
> > > > > >
> > > > > > Would it make sense to have a flag on the interface (or
> > > > > > globally), set when applying the ACL, that indicates the
> > > > > > desired behavior when the ACL is empty or non-existent? At the
> > > > > > moment to me it seems logical that this is the same behavior
> > > > > > as when matching falls off the end of the ACL.
> > > > > >
> > > > > > Chris.
> > >

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-16 Thread Marco Varlese
On Fri, 2017-06-16 at 15:12 +0200, Andrew   Yourtchenko wrote:
> On 6/16/17, Marco Varlese <marco.varl...@suse.com> wrote:
> > 
> > On Thu, 2017-06-15 at 14:22 +0200, Andrew   Yourtchenko wrote:
> > > 
> > > After a bit more thinking - there is a way that should take care of both:
> > > 
> > > 1) What Chris wrote: have consistent behaviour with non-existent ACL
> > > as if the ACL matching fell off the end of the ACL: if an empty ACL is
> > > referenced, treat it as if it had entries but none of them had
> > > matched. Then we still hit the "default deny" if none of the applied
> > > ACLs match, and drop the packets - so it will be logical.
> > > 
> > > 2) What Marco wrote: when deleting an already referenced ACL, unapply
> > > it from all the places where it is applied.
> > > 
> > > It's a change in the behaviour in that the current behaviour is to
> > > have the empty ACL act as if it was "deny any any", and block the
> > > matching even if there is another ACL after it - but on the other hand
> > > this would take both viewpoints in mind...
> > I think this approach would still leave the system in an inconsistent state:
> > an
> > interface is basically assigned an ACL which does not exist in the system.
> > Also, the risk I see is if I later on create an ACL with the previously
> > used
> > index then an interface would see that ACL being applied automatically
> > (since
> > it's referenced). However, I may not want to have that ACL assigned to that
> > particular interface. Correct?
> > 
> > I think that the "deletion" of an ACL could see one of the two behaviours
> > below:
> > 1) the deletion of an ACL should be DENIED if that ACL is assigned to any
> > interface (probably the easier and safer approach);
> > 2) the deletion of an ACL should see a CASCADING effect onto the interfaces
> > which would be "cleaned up" of any references to that ACL;
> > 
> 
> Right, the (2) was what I was trying to suggest to do...
> 
> > 
> > I think (1) is a very good way of solving the initial problem since it
> > works
> > nicely if you manage VPP directly (e.g. via command-line) and if you use a
> > controller. In the latter, the controller can react on the "error" returned
> > by
> > the acl_del API because that ACL is assigned somewhere.
> > 
> 
> ...but the (1) is another good option to me.
> 
> So it seems we are converging on (1) ?
I would go with (1)...
> 
> --a
> 
> 
> 
> > 
> > 
> > Cheers,
> > Marco
> > 
> > > 
> > > 
> > > What do you think ?
> > > 
> > > --a
> > > 
> > > On 6/9/17, Andrew   Yourtchenko <ayour...@gmail.com> wrote:
> > > > 
> > > > 
> > > > Assuming the only change is to effectively have
> > > > "unbind_acl_from_everywhere; delete_acl" instead of "delete_acl",
> > > > maybe it would be best to tackle that post-17.07 with a separate API
> > > > message acl_del_and_unbind or similar ?
> > > > 
> > > > I feel a beet wary of adding more hidden state (even though the
> > > > reflected sessions table does provide already plenty of it :)
> > > > 
> > > > --a
> > > > 
> > > > On 6/9/17, Luke, Chris <chris_l...@comcast.com> wrote:
> > > > > 
> > > > > 
> > > > > Would it make sense to have a flag on the interface (or globally),
> > > > > set
> > > > > when
> > > > > applying the ACL, that indicates the desired behavior when the ACL is
> > > > > empty
> > > > > or non-existent? At the moment to me it seems logical that this is
> > > > > the
> > > > > same
> > > > > behavior as when matching falls off the end of the ACL.
> > > > > 
> > > > > Chris.
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > -Original Message-
> > > > > > From: vpp-dev-boun...@lists.fd.io
> > > > > > [mailto:vpp-dev-boun...@lists.fd.io]
> > > > > > On
> > > > > > Behalf Of Andrew ?? Yourtchenko
> > > > > > Sent: Friday, June 9, 2017 7:53
> > > > > > To: Marco Varlese <marco.varl...@suse.com>
> > > > > > Cc: vpp-dev@lists.fd.io
> 

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-16 Thread Andrew  Yourtchenko
On 6/16/17, Marco Varlese <marco.varl...@suse.com> wrote:
> On Thu, 2017-06-15 at 14:22 +0200, Andrew   Yourtchenko wrote:
>> After a bit more thinking - there is a way that should take care of both:
>>
>> 1) What Chris wrote: have consistent behaviour with non-existent ACL
>> as if the ACL matching fell off the end of the ACL: if an empty ACL is
>> referenced, treat it as if it had entries but none of them had
>> matched. Then we still hit the "default deny" if none of the applied
>> ACLs match, and drop the packets - so it will be logical.
>>
>> 2) What Marco wrote: when deleting an already referenced ACL, unapply
>> it from all the places where it is applied.
>>
>> It's a change in the behaviour in that the current behaviour is to
>> have the empty ACL act as if it was "deny any any", and block the
>> matching even if there is another ACL after it - but on the other hand
>> this would take both viewpoints in mind...
> I think this approach would still leave the system in an inconsistent state:
> an
> interface is basically assigned an ACL which does not exist in the system.
> Also, the risk I see is if I later on create an ACL with the previously
> used
> index then an interface would see that ACL being applied automatically
> (since
> it's referenced). However, I may not want to have that ACL assigned to that
> particular interface. Correct?
>
> I think that the "deletion" of an ACL could see one of the two behaviours
> below:
> 1) the deletion of an ACL should be DENIED if that ACL is assigned to any
> interface (probably the easier and safer approach);
> 2) the deletion of an ACL should see a CASCADING effect onto the interfaces
> which would be "cleaned up" of any references to that ACL;
>

Right, the (2) was what I was trying to suggest to do...

> I think (1) is a very good way of solving the initial problem since it
> works
> nicely if you manage VPP directly (e.g. via command-line) and if you use a
> controller. In the latter, the controller can react on the "error" returned
> by
> the acl_del API because that ACL is assigned somewhere.
>

...but the (1) is another good option to me.

So it seems we are converging on (1) ?

--a



>
> Cheers,
> Marco
>
>>
>> What do you think ?
>>
>> --a
>>
>> On 6/9/17, Andrew   Yourtchenko <ayour...@gmail.com> wrote:
>> >
>> > Assuming the only change is to effectively have
>> > "unbind_acl_from_everywhere; delete_acl" instead of "delete_acl",
>> > maybe it would be best to tackle that post-17.07 with a separate API
>> > message acl_del_and_unbind or similar ?
>> >
>> > I feel a beet wary of adding more hidden state (even though the
>> > reflected sessions table does provide already plenty of it :)
>> >
>> > --a
>> >
>> > On 6/9/17, Luke, Chris <chris_l...@comcast.com> wrote:
>> > >
>> > > Would it make sense to have a flag on the interface (or globally),
>> > > set
>> > > when
>> > > applying the ACL, that indicates the desired behavior when the ACL is
>> > > empty
>> > > or non-existent? At the moment to me it seems logical that this is
>> > > the
>> > > same
>> > > behavior as when matching falls off the end of the ACL.
>> > >
>> > > Chris.
>> > >
>> > > >
>> > > > -Original Message-
>> > > > From: vpp-dev-boun...@lists.fd.io
>> > > > [mailto:vpp-dev-boun...@lists.fd.io]
>> > > > On
>> > > > Behalf Of Andrew ?? Yourtchenko
>> > > > Sent: Friday, June 9, 2017 7:53
>> > > > To: Marco Varlese <marco.varl...@suse.com>
>> > > > Cc: vpp-dev@lists.fd.io
>> > > > Subject: Re: [vpp-dev] Bind / Unbind of ACL
>> > > >
>> > > > Hi Marco,
>> > > >
>> > > > Yes, this works as expected, assuming after deletion *all* the
>> > > > traffic
>> > > > is
>> > > > denied, rather than just the SSH traffic.
>> > > >
>> > > > If you apply to an interface the ACL# that does not exist, that is
>> > > > the
>> > > > same as if
>> > > > there was an ACL with just the "deny all" semantics, to avoid the
>> > > > perception
>> > > > that a given policy is enforced when it isn't - so I erred on the
>> > > > side
>> > > > of
&

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-16 Thread Marco Varlese
On Thu, 2017-06-15 at 14:22 +0200, Andrew   Yourtchenko wrote:
> After a bit more thinking - there is a way that should take care of both:
> 
> 1) What Chris wrote: have consistent behaviour with non-existent ACL
> as if the ACL matching fell off the end of the ACL: if an empty ACL is
> referenced, treat it as if it had entries but none of them had
> matched. Then we still hit the "default deny" if none of the applied
> ACLs match, and drop the packets - so it will be logical.
> 
> 2) What Marco wrote: when deleting an already referenced ACL, unapply
> it from all the places where it is applied.
> 
> It's a change in the behaviour in that the current behaviour is to
> have the empty ACL act as if it was "deny any any", and block the
> matching even if there is another ACL after it - but on the other hand
> this would take both viewpoints in mind...
I think this approach would still leave the system in an inconsistent state: an
interface is basically assigned an ACL which does not exist in the system.
Also, the risk I see is if I later on create an ACL with the previously used
index then an interface would see that ACL being applied automatically (since
it's referenced). However, I may not want to have that ACL assigned to that
particular interface. Correct?

I think that the "deletion" of an ACL could see one of the two behaviours below:
1) the deletion of an ACL should be DENIED if that ACL is assigned to any
interface (probably the easier and safer approach);
2) the deletion of an ACL should see a CASCADING effect onto the interfaces
which would be "cleaned up" of any references to that ACL;

I think (1) is a very good way of solving the initial problem since it works
nicely if you manage VPP directly (e.g. via command-line) and if you use a
controller. In the latter, the controller can react on the "error" returned by
the acl_del API because that ACL is assigned somewhere.


Cheers,
Marco

> 
> What do you think ?
> 
> --a
> 
> On 6/9/17, Andrew   Yourtchenko <ayour...@gmail.com> wrote:
> > 
> > Assuming the only change is to effectively have
> > "unbind_acl_from_everywhere; delete_acl" instead of "delete_acl",
> > maybe it would be best to tackle that post-17.07 with a separate API
> > message acl_del_and_unbind or similar ?
> > 
> > I feel a beet wary of adding more hidden state (even though the
> > reflected sessions table does provide already plenty of it :)
> > 
> > --a
> > 
> > On 6/9/17, Luke, Chris <chris_l...@comcast.com> wrote:
> > > 
> > > Would it make sense to have a flag on the interface (or globally), set
> > > when
> > > applying the ACL, that indicates the desired behavior when the ACL is
> > > empty
> > > or non-existent? At the moment to me it seems logical that this is the
> > > same
> > > behavior as when matching falls off the end of the ACL.
> > > 
> > > Chris.
> > > 
> > > > 
> > > > -Original Message-
> > > > From: vpp-dev-boun...@lists.fd.io [mailto:vpp-dev-boun...@lists.fd.io]
> > > > On
> > > > Behalf Of Andrew ?? Yourtchenko
> > > > Sent: Friday, June 9, 2017 7:53
> > > > To: Marco Varlese <marco.varl...@suse.com>
> > > > Cc: vpp-dev@lists.fd.io
> > > > Subject: Re: [vpp-dev] Bind / Unbind of ACL
> > > > 
> > > > Hi Marco,
> > > > 
> > > > Yes, this works as expected, assuming after deletion *all* the traffic
> > > > is
> > > > denied, rather than just the SSH traffic.
> > > > 
> > > > If you apply to an interface the ACL# that does not exist, that is the
> > > > same as if
> > > > there was an ACL with just the "deny all" semantics, to avoid the
> > > > perception
> > > > that a given policy is enforced when it isn't - so I erred on the side
> > > > of
> > > > caution.
> > > > 
> > > > The way to remove the ACL: you would ensure the ACL is not applied to
> > > > the
> > > > interface(s) first, then remove the ACL (or replace it with a different
> > > > policy in-
> > > > place).
> > > > 
> > > > Alternatively, you can just replace the existing ACL in-place with
> > > > "permit
> > > > any"
> > > > for IPv4 and IPv6 - this way you explicitly state that there is a policy
> > > > to permit
> > > > all the traffic.
> > > > 
> > > > I've been bitten myself and seen several times in m

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-15 Thread Andrew  Yourtchenko
On 6/15/17, Luke, Chris <chris_l...@comcast.com> wrote:
> I’m mostly agnostic with 2; an alternate behavior might be to refuse to
> delete an ACL if it is referenced anywhere. I’m more concerned with
> consistency with this sort of operation across VPP which I feel is more
> important, though no other examples spring to mind this early in the
> morning!

Right... From what I see if e.g. I set the l2 classify lookup table #
for the interface and then free the table itself, node function still
goes into a given index, so I suspect the consequences of this move
would be visible quite quickly

So for now in the new bihash-based ACL lookup code I went ahead with
this change of behaviour (I've added you to the draft so you can see
it), and we can later align the linear lookup as well to match with
this behaviour.

--a

>
> Chris.
>
> On 6/15/17, 08:22, "Andrew   Yourtchenko" <ayour...@gmail.com> wrote:
>
> After a bit more thinking - there is a way that should take care of
> both:
>
> 1) What Chris wrote: have consistent behaviour with non-existent ACL
> as if the ACL matching fell off the end of the ACL: if an empty ACL is
> referenced, treat it as if it had entries but none of them had
> matched. Then we still hit the "default deny" if none of the applied
> ACLs match, and drop the packets - so it will be logical.
>
> 2) What Marco wrote: when deleting an already referenced ACL, unapply
> it from all the places where it is applied.
>
> It's a change in the behaviour in that the current behaviour is to
> have the empty ACL act as if it was "deny any any", and block the
> matching even if there is another ACL after it - but on the other hand
> this would take both viewpoints in mind...
>
> What do you think ?
>
> --a
>
> On 6/9/17, Andrew   Yourtchenko <ayour...@gmail.com> wrote:
> > Assuming the only change is to effectively have
> > "unbind_acl_from_everywhere; delete_acl" instead of "delete_acl",
> > maybe it would be best to tackle that post-17.07 with a separate API
> > message acl_del_and_unbind or similar ?
> >
> > I feel a beet wary of adding more hidden state (even though the
> > reflected sessions table does provide already plenty of it :)
> >
> > --a
> >
> > On 6/9/17, Luke, Chris <chris_l...@comcast.com> wrote:
> >> Would it make sense to have a flag on the interface (or globally),
> set
> >> when
> >> applying the ACL, that indicates the desired behavior when the ACL
> is
> >> empty
> >> or non-existent? At the moment to me it seems logical that this is
> the
> >> same
> >> behavior as when matching falls off the end of the ACL.
> >>
> >> Chris.
> >>
>     >>> -Original Message-
> >>> From: vpp-dev-boun...@lists.fd.io
> [mailto:vpp-dev-boun...@lists.fd.io]
> >>> On
> >>> Behalf Of Andrew ?? Yourtchenko
> >>> Sent: Friday, June 9, 2017 7:53
> >>> To: Marco Varlese <marco.varl...@suse.com>
> >>> Cc: vpp-dev@lists.fd.io
> >>> Subject: Re: [vpp-dev] Bind / Unbind of ACL
> >>>
> >>> Hi Marco,
> >>>
> >>> Yes, this works as expected, assuming after deletion *all* the
> traffic
> >>> is
> >>> denied, rather than just the SSH traffic.
> >>>
> >>> If you apply to an interface the ACL# that does not exist, that is
> the
> >>> same as if
> >>> there was an ACL with just the "deny all" semantics, to avoid the
> >>> perception
> >>> that a given policy is enforced when it isn't - so I erred on the
> side
> >>> of
> >>> caution.
> >>>
> >>> The way to remove the ACL: you would ensure the ACL is not applied
> to
> >>> the
> >>> interface(s) first, then remove the ACL (or replace it with a
> different
> >>> policy in-
> >>> place).
> >>>
> >>> Alternatively, you can just replace the existing ACL in-place with
> >>> "permit
> >>> any"
> >>> for IPv4 and IPv6 - this way you explicitly state that there is a
> policy
> >>> to permit
> >>> all the traffic.
> >>>
> >>> I've been bitten myself and seen several tim

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-15 Thread Luke, Chris
I’m mostly agnostic with 2; an alternate behavior might be to refuse to delete 
an ACL if it is referenced anywhere. I’m more concerned with consistency with 
this sort of operation across VPP which I feel is more important, though no 
other examples spring to mind this early in the morning!

Chris.

On 6/15/17, 08:22, "Andrew   Yourtchenko" <ayour...@gmail.com> wrote:

After a bit more thinking - there is a way that should take care of both:

1) What Chris wrote: have consistent behaviour with non-existent ACL
as if the ACL matching fell off the end of the ACL: if an empty ACL is
referenced, treat it as if it had entries but none of them had
matched. Then we still hit the "default deny" if none of the applied
ACLs match, and drop the packets - so it will be logical.

2) What Marco wrote: when deleting an already referenced ACL, unapply
it from all the places where it is applied.

It's a change in the behaviour in that the current behaviour is to
have the empty ACL act as if it was "deny any any", and block the
matching even if there is another ACL after it - but on the other hand
this would take both viewpoints in mind...

What do you think ?

--a

On 6/9/17, Andrew   Yourtchenko <ayour...@gmail.com> wrote:
> Assuming the only change is to effectively have
> "unbind_acl_from_everywhere; delete_acl" instead of "delete_acl",
> maybe it would be best to tackle that post-17.07 with a separate API
> message acl_del_and_unbind or similar ?
>
> I feel a beet wary of adding more hidden state (even though the
> reflected sessions table does provide already plenty of it :)
>
> --a
>
> On 6/9/17, Luke, Chris <chris_l...@comcast.com> wrote:
>> Would it make sense to have a flag on the interface (or globally), set
>> when
>> applying the ACL, that indicates the desired behavior when the ACL is
>> empty
>> or non-existent? At the moment to me it seems logical that this is the
>> same
>> behavior as when matching falls off the end of the ACL.
>>
>> Chris.
>>
>>> -Original Message-
>>> From: vpp-dev-boun...@lists.fd.io [mailto:vpp-dev-boun...@lists.fd.io]
>>> On
    >>> Behalf Of Andrew ?? Yourtchenko
>>> Sent: Friday, June 9, 2017 7:53
>>> To: Marco Varlese <marco.varl...@suse.com>
>>> Cc: vpp-dev@lists.fd.io
>>> Subject: Re: [vpp-dev] Bind / Unbind of ACL
>>>
>>> Hi Marco,
>>>
>>> Yes, this works as expected, assuming after deletion *all* the traffic
>>> is
>>> denied, rather than just the SSH traffic.
>>>
>>> If you apply to an interface the ACL# that does not exist, that is the
>>> same as if
>>> there was an ACL with just the "deny all" semantics, to avoid the
>>> perception
>>> that a given policy is enforced when it isn't - so I erred on the side
>>> of
>>> caution.
>>>
>>> The way to remove the ACL: you would ensure the ACL is not applied to
>>> the
>>> interface(s) first, then remove the ACL (or replace it with a different
>>> policy in-
>>> place).
>>>
>>> Alternatively, you can just replace the existing ACL in-place with
>>> "permit
>>> any"
>>> for IPv4 and IPv6 - this way you explicitly state that there is a policy
>>> to permit
>>> all the traffic.
>>>
>>> I've been bitten myself and seen several times in my career when an
>>> applied
>>> but non-existent ACL caused problems later on, in the worst possible
>>> moment. The current behaviour IMHO makes the config discrepancy clear -
>>> what do you think ?
>>>
>>> --a
>>>
>>> On 6/9/17, Marco Varlese <marco.varl...@suse.com> wrote:
>>> > Hi,
>>> >
>>> > I am trying the ACL functionality and I found a "strange" behaviour.
>>> >
>>> > The steps I follow to use an ACL are:
>>> > * I create an ACL to deny SSH traffic between VMs (via the
>>> 'acl_add_replace'
>>> > function)
>>> > * Set that ACL to the interfaces involved (via the
>>> > 'acl_interface_set_acl_list'
>>> > function)
>>> >
>>> > After performing the 

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-15 Thread Andrew  Yourtchenko
After a bit more thinking - there is a way that should take care of both:

1) What Chris wrote: have consistent behaviour with non-existent ACL
as if the ACL matching fell off the end of the ACL: if an empty ACL is
referenced, treat it as if it had entries but none of them had
matched. Then we still hit the "default deny" if none of the applied
ACLs match, and drop the packets - so it will be logical.

2) What Marco wrote: when deleting an already referenced ACL, unapply
it from all the places where it is applied.

It's a change in the behaviour in that the current behaviour is to
have the empty ACL act as if it was "deny any any", and block the
matching even if there is another ACL after it - but on the other hand
this would take both viewpoints in mind...

What do you think ?

--a

On 6/9/17, Andrew   Yourtchenko <ayour...@gmail.com> wrote:
> Assuming the only change is to effectively have
> "unbind_acl_from_everywhere; delete_acl" instead of "delete_acl",
> maybe it would be best to tackle that post-17.07 with a separate API
> message acl_del_and_unbind or similar ?
>
> I feel a beet wary of adding more hidden state (even though the
> reflected sessions table does provide already plenty of it :)
>
> --a
>
> On 6/9/17, Luke, Chris <chris_l...@comcast.com> wrote:
>> Would it make sense to have a flag on the interface (or globally), set
>> when
>> applying the ACL, that indicates the desired behavior when the ACL is
>> empty
>> or non-existent? At the moment to me it seems logical that this is the
>> same
>> behavior as when matching falls off the end of the ACL.
>>
>> Chris.
>>
>>> -Original Message-
>>> From: vpp-dev-boun...@lists.fd.io [mailto:vpp-dev-boun...@lists.fd.io]
>>> On
>>> Behalf Of Andrew ?? Yourtchenko
>>> Sent: Friday, June 9, 2017 7:53
>>> To: Marco Varlese <marco.varl...@suse.com>
>>> Cc: vpp-dev@lists.fd.io
>>> Subject: Re: [vpp-dev] Bind / Unbind of ACL
>>>
>>> Hi Marco,
>>>
>>> Yes, this works as expected, assuming after deletion *all* the traffic
>>> is
>>> denied, rather than just the SSH traffic.
>>>
>>> If you apply to an interface the ACL# that does not exist, that is the
>>> same as if
>>> there was an ACL with just the "deny all" semantics, to avoid the
>>> perception
>>> that a given policy is enforced when it isn't - so I erred on the side
>>> of
>>> caution.
>>>
>>> The way to remove the ACL: you would ensure the ACL is not applied to
>>> the
>>> interface(s) first, then remove the ACL (or replace it with a different
>>> policy in-
>>> place).
>>>
>>> Alternatively, you can just replace the existing ACL in-place with
>>> "permit
>>> any"
>>> for IPv4 and IPv6 - this way you explicitly state that there is a policy
>>> to permit
>>> all the traffic.
>>>
>>> I've been bitten myself and seen several times in my career when an
>>> applied
>>> but non-existent ACL caused problems later on, in the worst possible
>>> moment. The current behaviour IMHO makes the config discrepancy clear -
>>> what do you think ?
>>>
>>> --a
>>>
>>> On 6/9/17, Marco Varlese <marco.varl...@suse.com> wrote:
>>> > Hi,
>>> >
>>> > I am trying the ACL functionality and I found a "strange" behaviour.
>>> >
>>> > The steps I follow to use an ACL are:
>>> > * I create an ACL to deny SSH traffic between VMs (via the
>>> 'acl_add_replace'
>>> > function)
>>> > * Set that ACL to the interfaces involved (via the
>>> > 'acl_interface_set_acl_list'
>>> > function)
>>> >
>>> > After performing the above steps the traffic was correctly being
>>> > blocked.
>>> >
>>> > However, when I decided to enable the SSH traffic again, I simply
>>> > deleted the ACL (via the 'acl_del' function) with the consequence
>>> > though that the traffic was still being denied.
>>> >
>>> > Is this behaviour correct?
>>> > If so what would be the right way to unset hence disable a given ACL
>>> > from an interface (or multiple)?
>>> >
>>> >
>>> > Thanks,
>>> > Marco
>>> >
>>> > ___
>>> > vpp-dev mailing list
>>> > vpp-dev@lists.fd.io
>>> > https://lists.fd.io/mailman/listinfo/vpp-dev
>>> ___
>>> vpp-dev mailing list
>>> vpp-dev@lists.fd.io
>>> https://lists.fd.io/mailman/listinfo/vpp-dev
>>
>>
>
___
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-09 Thread Marco Varlese
On Fri, 2017-06-09 at 14:27 +0200, Andrew   Yourtchenko wrote:
> Hi Marco,
> 
> On 6/9/17, Marco Varlese  wrote:
> > 
> > Hi Andrew,
> > 
> > On Fri, 2017-06-09 at 13:53 +0200, Andrew   Yourtchenko wrote:
> > > 
> > > Hi Marco,
> > > 
> > > Yes, this works as expected, assuming after deletion *all* the traffic
> > > is denied, rather than just the SSH traffic.
> > > 
> > > If you apply to an interface the ACL# that does not exist, that is the
> > > same as if there was an ACL with just the "deny all" semantics, to
> > > avoid the perception that a given policy is enforced when it isn't -
> > > so I erred on the side of caution.
> > > 
> > > The way to remove the ACL: you would ensure the ACL is not applied to
> > > the interface(s) first, then remove the ACL (or replace it with a
> > > different policy in-place).
> > Ok, which function would allow me to unset the ACL from an interface?
> > I see on the documentation that 'acl_interface_add_del' is marked as "not
> > recommended" hence I wonder whether it will soon be marked as deprecated
> > and
> > eventually removed.
> 
> I encourage the users to use the acl_interface_set_acl_list mostly
> because it has a clearer (IMHO) semantics - removing all the ACLs
> means simply setting the empty list for in+out...
> 
> As for the deprecation - I think it will be a while, if at all. And of
> course if the users say "no, we find it useful and we need it", then
> it won't be deprecated at all :-)
> 
> > 
> > 
> > > 
> > > 
> > > Alternatively, you can just replace the existing ACL in-place with
> > > "permit any" for IPv4 and IPv6 - this way you explicitly state that
> > > there is a policy to permit all the traffic.
> > > 
> > > I've been bitten myself and seen several times in my career when an
> > > applied but non-existent ACL caused problems later on, in the worst
> > > possible moment. The current behaviour IMHO makes the config
> > > discrepancy clear - what do you think ?
> > In the past, when I had to work on ACL implementation, I approached the
> > solution
> > differently: an ACL (whether deny or permit) which is referenced (e.g.
> > applied
> > to one or multiple interfaces) if deleted would see a cascading effect
> > (please,
> > allow me the expression) of that deletion onto any interface which was
> > referencing it.
> > 
> > The "problem" I see - with the current approach - is that once an ACL is
> > deleted
> > it's much harder to understand / debug why a given flow is either permitted
> > or
> > not (depending on the action of the ACL). If you have hundreds or thousands
> > of
> > ACL/rules then things get complicated very quickly.
> > Instead, by applying the "cascading" effect hence freeing the interfaces
> > from
> > the previous behaviour, things would have a 1:1 mapping between what you see
> > in
> > configuration (acl_dump) with the flows you see on the network.
> 
> True, this is also a valid approach, feel free to submit a gerrit
> doing this. :-)
> 
Well, I can obviously code it but I won't code something which was discarded at
design time or won't ever be accepted.
If you feel it is a better approach then the current one then I will spend time
on it...

> I will also add you to a draft of my work-in-progress quicker lookup
> so you can see how it all interacts (and indeed will be happy to hear
> your feedback on that one too!)
> 
> I think the definition of policy and its application from the control
> plane standpoint  in this day  should be automated - so then the
> control plane would have to do this housekeeping already anyway
> internally, thus unapplying the ACL is just (in my understanding) just
> a single call. But I can see the benefits of the automatic cleanup
> too, so I am happy with either way. (especially since this does not
> break the things for the clients that do the unapply already).
> 
> --a
> 
> > 
> > 
> > > 
> > > 
> > > --a
> > Cheers,
> > Marco
> > 
> > > 
> > > 
> > > On 6/9/17, Marco Varlese  wrote:
> > > > 
> > > > 
> > > > Hi,
> > > > 
> > > > I am trying the ACL functionality and I found a "strange" behaviour.
> > > > 
> > > > The steps I follow to use an ACL are:
> > > > * I create an ACL to deny SSH traffic between VMs (via the
> > > > 'acl_add_replace'
> > > > function)
> > > > * Set that ACL to the interfaces involved (via the
> > > > 'acl_interface_set_acl_list'
> > > > function)
> > > > 
> > > > After performing the above steps the traffic was correctly being
> > > > blocked.
> > > > 
> > > > However, when I decided to enable the SSH traffic again, I simply
> > > > deleted
> > > > the
> > > > ACL (via the 'acl_del' function) with the consequence though that the
> > > > traffic
> > > > was still being denied.
> > > > 
> > > > Is this behaviour correct?
> > > > If so what would be the right way to unset hence disable a given ACL
> > > > from an
> > > > interface (or multiple)?
> > > > 
> > > > 
> > > > Thanks,
> > > > Marco
> > > > 
> > > > 

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-09 Thread Andrew  Yourtchenko
Assuming the only change is to effectively have
"unbind_acl_from_everywhere; delete_acl" instead of "delete_acl",
maybe it would be best to tackle that post-17.07 with a separate API
message acl_del_and_unbind or similar ?

I feel a beet wary of adding more hidden state (even though the
reflected sessions table does provide already plenty of it :)

--a

On 6/9/17, Luke, Chris <chris_l...@comcast.com> wrote:
> Would it make sense to have a flag on the interface (or globally), set when
> applying the ACL, that indicates the desired behavior when the ACL is empty
> or non-existent? At the moment to me it seems logical that this is the same
> behavior as when matching falls off the end of the ACL.
>
> Chris.
>
>> -Original Message-
>> From: vpp-dev-boun...@lists.fd.io [mailto:vpp-dev-boun...@lists.fd.io] On
>> Behalf Of Andrew ?? Yourtchenko
>> Sent: Friday, June 9, 2017 7:53
>> To: Marco Varlese <marco.varl...@suse.com>
>> Cc: vpp-dev@lists.fd.io
>> Subject: Re: [vpp-dev] Bind / Unbind of ACL
>>
>> Hi Marco,
>>
>> Yes, this works as expected, assuming after deletion *all* the traffic is
>> denied, rather than just the SSH traffic.
>>
>> If you apply to an interface the ACL# that does not exist, that is the
>> same as if
>> there was an ACL with just the "deny all" semantics, to avoid the
>> perception
>> that a given policy is enforced when it isn't - so I erred on the side of
>> caution.
>>
>> The way to remove the ACL: you would ensure the ACL is not applied to the
>> interface(s) first, then remove the ACL (or replace it with a different
>> policy in-
>> place).
>>
>> Alternatively, you can just replace the existing ACL in-place with "permit
>> any"
>> for IPv4 and IPv6 - this way you explicitly state that there is a policy
>> to permit
>> all the traffic.
>>
>> I've been bitten myself and seen several times in my career when an
>> applied
>> but non-existent ACL caused problems later on, in the worst possible
>> moment. The current behaviour IMHO makes the config discrepancy clear -
>> what do you think ?
>>
>> --a
>>
>> On 6/9/17, Marco Varlese <marco.varl...@suse.com> wrote:
>> > Hi,
>> >
>> > I am trying the ACL functionality and I found a "strange" behaviour.
>> >
>> > The steps I follow to use an ACL are:
>> > * I create an ACL to deny SSH traffic between VMs (via the
>> 'acl_add_replace'
>> > function)
>> > * Set that ACL to the interfaces involved (via the
>> > 'acl_interface_set_acl_list'
>> > function)
>> >
>> > After performing the above steps the traffic was correctly being
>> > blocked.
>> >
>> > However, when I decided to enable the SSH traffic again, I simply
>> > deleted the ACL (via the 'acl_del' function) with the consequence
>> > though that the traffic was still being denied.
>> >
>> > Is this behaviour correct?
>> > If so what would be the right way to unset hence disable a given ACL
>> > from an interface (or multiple)?
>> >
>> >
>> > Thanks,
>> > Marco
>> >
>> > ___
>> > vpp-dev mailing list
>> > vpp-dev@lists.fd.io
>> > https://lists.fd.io/mailman/listinfo/vpp-dev
>> ___
>> vpp-dev mailing list
>> vpp-dev@lists.fd.io
>> https://lists.fd.io/mailman/listinfo/vpp-dev
>
>
___
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev


Re: [vpp-dev] Bind / Unbind of ACL

2017-06-09 Thread Luke, Chris
Would it make sense to have a flag on the interface (or globally), set when 
applying the ACL, that indicates the desired behavior when the ACL is empty or 
non-existent? At the moment to me it seems logical that this is the same 
behavior as when matching falls off the end of the ACL.

Chris.

> -Original Message-
> From: vpp-dev-boun...@lists.fd.io [mailto:vpp-dev-boun...@lists.fd.io] On
> Behalf Of Andrew ?? Yourtchenko
> Sent: Friday, June 9, 2017 7:53
> To: Marco Varlese <marco.varl...@suse.com>
> Cc: vpp-dev@lists.fd.io
> Subject: Re: [vpp-dev] Bind / Unbind of ACL
> 
> Hi Marco,
> 
> Yes, this works as expected, assuming after deletion *all* the traffic is
> denied, rather than just the SSH traffic.
> 
> If you apply to an interface the ACL# that does not exist, that is the same 
> as if
> there was an ACL with just the "deny all" semantics, to avoid the perception
> that a given policy is enforced when it isn't - so I erred on the side of 
> caution.
> 
> The way to remove the ACL: you would ensure the ACL is not applied to the
> interface(s) first, then remove the ACL (or replace it with a different 
> policy in-
> place).
> 
> Alternatively, you can just replace the existing ACL in-place with "permit 
> any"
> for IPv4 and IPv6 - this way you explicitly state that there is a policy to 
> permit
> all the traffic.
> 
> I've been bitten myself and seen several times in my career when an applied
> but non-existent ACL caused problems later on, in the worst possible
> moment. The current behaviour IMHO makes the config discrepancy clear -
> what do you think ?
> 
> --a
> 
> On 6/9/17, Marco Varlese <marco.varl...@suse.com> wrote:
> > Hi,
> >
> > I am trying the ACL functionality and I found a "strange" behaviour.
> >
> > The steps I follow to use an ACL are:
> > * I create an ACL to deny SSH traffic between VMs (via the
> 'acl_add_replace'
> > function)
> > * Set that ACL to the interfaces involved (via the
> > 'acl_interface_set_acl_list'
> > function)
> >
> > After performing the above steps the traffic was correctly being blocked.
> >
> > However, when I decided to enable the SSH traffic again, I simply
> > deleted the ACL (via the 'acl_del' function) with the consequence
> > though that the traffic was still being denied.
> >
> > Is this behaviour correct?
> > If so what would be the right way to unset hence disable a given ACL
> > from an interface (or multiple)?
> >
> >
> > Thanks,
> > Marco
> >
> > ___
> > vpp-dev mailing list
> > vpp-dev@lists.fd.io
> > https://lists.fd.io/mailman/listinfo/vpp-dev
> ___
> vpp-dev mailing list
> vpp-dev@lists.fd.io
> https://lists.fd.io/mailman/listinfo/vpp-dev

___
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev


Re: [vpp-dev] Bind / Unbind of ACL

2017-06-09 Thread Marco Varlese
Hi Andrew,

On Fri, 2017-06-09 at 13:53 +0200, Andrew   Yourtchenko wrote:
> Hi Marco,
> 
> Yes, this works as expected, assuming after deletion *all* the traffic
> is denied, rather than just the SSH traffic.
> 
> If you apply to an interface the ACL# that does not exist, that is the
> same as if there was an ACL with just the "deny all" semantics, to
> avoid the perception that a given policy is enforced when it isn't -
> so I erred on the side of caution.
> 
> The way to remove the ACL: you would ensure the ACL is not applied to
> the interface(s) first, then remove the ACL (or replace it with a
> different policy in-place).
Ok, which function would allow me to unset the ACL from an interface?
I see on the documentation that 'acl_interface_add_del' is marked as "not
recommended" hence I wonder whether it will soon be marked as deprecated and
eventually removed.

> 
> Alternatively, you can just replace the existing ACL in-place with
> "permit any" for IPv4 and IPv6 - this way you explicitly state that
> there is a policy to permit all the traffic.
> 
> I've been bitten myself and seen several times in my career when an
> applied but non-existent ACL caused problems later on, in the worst
> possible moment. The current behaviour IMHO makes the config
> discrepancy clear - what do you think ?
In the past, when I had to work on ACL implementation, I approached the solution
differently: an ACL (whether deny or permit) which is referenced (e.g. applied
to one or multiple interfaces) if deleted would see a cascading effect (please,
allow me the expression) of that deletion onto any interface which was
referencing it. 

The "problem" I see - with the current approach - is that once an ACL is deleted
it's much harder to understand / debug why a given flow is either permitted or
not (depending on the action of the ACL). If you have hundreds or thousands of
ACL/rules then things get complicated very quickly.
Instead, by applying the "cascading" effect hence freeing the interfaces from
the previous behaviour, things would have a 1:1 mapping between what you see in
configuration (acl_dump) with the flows you see on the network.

> 
> --a
Cheers,
Marco

> 
> On 6/9/17, Marco Varlese  wrote:
> > 
> > Hi,
> > 
> > I am trying the ACL functionality and I found a "strange" behaviour.
> > 
> > The steps I follow to use an ACL are:
> > * I create an ACL to deny SSH traffic between VMs (via the 'acl_add_replace'
> > function)
> > * Set that ACL to the interfaces involved (via the
> > 'acl_interface_set_acl_list'
> > function)
> > 
> > After performing the above steps the traffic was correctly being blocked.
> > 
> > However, when I decided to enable the SSH traffic again, I simply deleted
> > the
> > ACL (via the 'acl_del' function) with the consequence though that the
> > traffic
> > was still being denied.
> > 
> > Is this behaviour correct?
> > If so what would be the right way to unset hence disable a given ACL from an
> > interface (or multiple)?
> > 
> > 
> > Thanks,
> > Marco
> > 
> > ___
> > vpp-dev mailing list
> > vpp-dev@lists.fd.io
> > https://lists.fd.io/mailman/listinfo/vpp-dev
> 
___
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev

Re: [vpp-dev] Bind / Unbind of ACL

2017-06-09 Thread Andrew  Yourtchenko
Hi Marco,

Yes, this works as expected, assuming after deletion *all* the traffic
is denied, rather than just the SSH traffic.

If you apply to an interface the ACL# that does not exist, that is the
same as if there was an ACL with just the "deny all" semantics, to
avoid the perception that a given policy is enforced when it isn't -
so I erred on the side of caution.

The way to remove the ACL: you would ensure the ACL is not applied to
the interface(s) first, then remove the ACL (or replace it with a
different policy in-place).

Alternatively, you can just replace the existing ACL in-place with
"permit any" for IPv4 and IPv6 - this way you explicitly state that
there is a policy to permit all the traffic.

I've been bitten myself and seen several times in my career when an
applied but non-existent ACL caused problems later on, in the worst
possible moment. The current behaviour IMHO makes the config
discrepancy clear - what do you think ?

--a

On 6/9/17, Marco Varlese  wrote:
> Hi,
>
> I am trying the ACL functionality and I found a "strange" behaviour.
>
> The steps I follow to use an ACL are:
> * I create an ACL to deny SSH traffic between VMs (via the 'acl_add_replace'
> function)
> * Set that ACL to the interfaces involved (via the
> 'acl_interface_set_acl_list'
> function)
>
> After performing the above steps the traffic was correctly being blocked.
>
> However, when I decided to enable the SSH traffic again, I simply deleted
> the
> ACL (via the 'acl_del' function) with the consequence though that the
> traffic
> was still being denied.
>
> Is this behaviour correct?
> If so what would be the right way to unset hence disable a given ACL from an
> interface (or multiple)?
>
>
> Thanks,
> Marco
>
> ___
> vpp-dev mailing list
> vpp-dev@lists.fd.io
> https://lists.fd.io/mailman/listinfo/vpp-dev
___
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev


[vpp-dev] Bind / Unbind of ACL

2017-06-09 Thread Marco Varlese
Hi,

I am trying the ACL functionality and I found a "strange" behaviour.

The steps I follow to use an ACL are:
* I create an ACL to deny SSH traffic between VMs (via the 'acl_add_replace'
function)
* Set that ACL to the interfaces involved (via the 'acl_interface_set_acl_list'
function)

After performing the above steps the traffic was correctly being blocked.

However, when I decided to enable the SSH traffic again, I simply deleted the
ACL (via the 'acl_del' function) with the consequence though that the traffic
was still being denied.

Is this behaviour correct? 
If so what would be the right way to unset hence disable a given ACL from an
interface (or multiple)?


Thanks,
Marco

___
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev