Folks,

So, yeah, I was just blind-sided by an API change in the ACL code.
Not to name names, or anything by it was

    commit 36ea2d6d3a67a60534a7c2b58551688858a1ce7f

    One armed NAT (VPP-1035)

    Use a single physical interface in order to accomplish NAT44/NAT64.

That patch also introduced many manipulations of the field "is_inside".
While it continues to use a field named "is_inside", it has now effectively
become a 4-state flag field:

$ git grep NAT_INTERFACE_FLAG_IS_
src/plugins/nat/nat.c:                  i->flags &=
~NAT_INTERFACE_FLAG_IS_INSIDE;
src/plugins/nat/nat.c:                  i->flags &=
~NAT_INTERFACE_FLAG_IS_OUTSIDE;
src/plugins/nat/nat.c:    i->flags |= NAT_INTERFACE_FLAG_IS_INSIDE;
src/plugins/nat/nat.c:    i->flags |= NAT_INTERFACE_FLAG_IS_OUTSIDE;
src/plugins/nat/nat.c:    i->flags |= NAT_INTERFACE_FLAG_IS_INSIDE;
src/plugins/nat/nat.c:    i->flags |= NAT_INTERFACE_FLAG_IS_OUTSIDE;
src/plugins/nat/nat.h:#define NAT_INTERFACE_FLAG_IS_INSIDE 1
src/plugins/nat/nat.h:#define NAT_INTERFACE_FLAG_IS_OUTSIDE 2
src/plugins/nat/nat.h:#define nat_interface_is_inside(i) i->flags &
NAT_INTERFACE_FLAG_IS_INSIDE
src/plugins/nat/nat.h:#define nat_interface_is_outside(i) i->flags &
NAT_INTERFACE_FLAG_IS_OUTSIDE
src/plugins/nat/nat64.c:        interface->flags |=
NAT_INTERFACE_FLAG_IS_INSIDE;
src/plugins/nat/nat64.c:        interface->flags |=
NAT_INTERFACE_FLAG_IS_OUTSIDE;
src/plugins/nat/nat64.c:          is_inside ? ~NAT_INTERFACE_FLAG_IS_INSIDE
:
src/plugins/nat/nat64.c:          ~NAT_INTERFACE_FLAG_IS_OUTSIDE;

Which is all pretty much fine...  Except that it continues to use a
field named "is_inside", and when passed _back_ to the Client in a
DUMP/DETAIL,
message, it is a tri-state value:
    is_inside == 0 <==> "outside"
    is_inside == 1 <==> "inside"
    is_inside == 2 <==> "both"

Which is where the unannounced, yet visible API change took place.

Here's the whole API file change here:

diff --git a/src/plugins/nat/nat.api b/src/plugins/nat/nat.api
index 8418757..fe40821 100644
--- a/src/plugins/nat/nat.api
+++ b/src/plugins/nat/nat.api
@@ -827,7 +827,7 @@ define nat44_interface_dump {

 /** \brief NAT44 interface details response
     @param context - sender context, to match reply w/ request
-    @param is_inside - 1 if inside, 0 if outside
+    @param is_inside - 1 if inside, 0 if outside, 2 if inside and outside
     @param sw_if_index - software index of the interface
 */

Call me a old grumpy whiner, but can we please post something to
the list about these sorts of changes?

And can we please rename that field away from "is_inside" to something
more descriptive and *less* misleading now?

I understand that this quad-state is really a combination of "on a list or
not"
and "one of three states if it is on a list", but it really would be much
clearer
if the flags field was used as a real 4-state (2-bit) flag field and the
INSIDE and OUTSIDE bits were clearly declared as such using 0x1 and 0x2
and 0x3 was used at the API to indicate both IN and OUT.

Blah.

jdl

PS -- Yes, I can write that patch if it would be accepted...
_______________________________________________
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev

Reply via email to