On Fri, Dec 8, 2017 at 12:34 PM, Andrew Yourtchenko <[email protected]>
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
[email protected]
https://lists.fd.io/mailman/listinfo/vpp-dev
