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