On Fri, Dec 8, 2017 at 12:34 PM, Andrew Yourtchenko <ayour...@gmail.com>
wrote:

> Jon,
>
> Do you have an api trace you could throw in my direction unicast ?
>

I'd have to work at that quite a bit as I am using a totally new and
different UI to drive these API calls...  I can outline it for you:

    macip_index = create macip acl with one rule "permit"
    call interface bind (some sw_if_index, macip_index)

    show the classifier tables
    show the MACIP interface bindings
    show the MACIP acls

    call macip_acl_add_replace(macip_index, "change rule to deny")

    show the new classifier tables (look ok)
    show the MACIP interface bindings (look ok)
    show the MACIP acls (look ok except the {ip4,ip6,l2}_table_index values
are 0; should be 3?)


> Macip_add_replace call was added later than most ACL plugins, (after
> realizing the convenience of this approach in the policy ACLs vs the
> unapply/delete acl/readd acl/reapply and for consistency), via commit
> c29940c58de3e44c0c1dd5c4eda5e0268d963b14
>

I know.  In fact, I had a different bug reported against our UI due
to the old way of removing the rules and then not getting them
re-applied upon addition correctly.

So when I saw the new API call (add_replace), I jumped on it!

I think I've just re-discovered the same underlying problem just
two layers deeper into the MACIP implementation now. :-/



> That change modified the existing “macip_acl_add_list” routine to also get
> the replace semantics, by deleting and then recreating the classifier
> tables anew.
>

I've just read through the macip_create_classifier_tables(), so yeah...


> This doesn’t work very well if the classifier tables are applied
> somewhere: they are removed, but the new tables are not applied
> automatically.
>

Right.  That's what I think I am seeing.


> We need to iterate the am->macip_acl_by_sw_if_index and for interfaces
> which have the acl in question applied, reapply these new tables to these
> interfaces so the already active macip acl remained active.
>

Ah, I see....


> This is a bug. Sorry for that.
>

No worries.  I've written a bug or two in my day...


>  Since there were also tests in that commit which ideally should have
> caught this, i will also need to take a double check at what is going on
> there (my guess missing negative tests), and fix it too, but not sure I can
> pull off thumbtyping in a vi session,
>

I see your problem here. No one should have to use vi. :-)


> so I will add you to the changerequest with the fix once I have it, hope
> in a day or so.
>

Excellent!

Thank you!
jdl
_______________________________________________
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev

Reply via email to