vpp: private plugin that adds npol matched rule to acl trace

[hedibouattour]

This adds the rule that matched in npol policy engine to the trace shown by acl node.

When a custom access policy matches a packet, store the matched rule id
in the session so subsequent packets of the same flow can retrieve it
without re-running the match, avoiding repeated policy evaluation overhead.
The rule id is propagated to the trace for debuggability. The new field
fits within existing reserved space in fa_session_t, keeping the struct
at 128 bytes with no cache-line impact.

Example of a deny policy hit:

Packet 49

00:10:28:590232: af-packet-input

  af_packet: hw_if_index 1 rx-queue 0 next-index 4

    tpacket2_hdr:

      status 0x20000001 len 94 snaplen 94 mac 76 net 90

      sec 0x69a6d6d0 nsec 0x2a0e9bb vlan 0 vlan_tpid 0

    vnet-hdr:

......
......
......
00:10:28:590248: ip4-rewrite

  tx_sw_if_index 10 dpo-idx 36 : ipv4 via 0.0.0.0 tun2: mtu:9216 next:10 flags:[features ] flow hash: 0x00000000

  00000000: 4500003cb3ea40003e06720c0b0000c40b000002de181f909445acd700000000

  00000020: a002fd5c8fbb0000020405820402080a539c11f20000000001030307

00:10:28:590249: acl-plugin-out-ip4-fa

  acl-plugin: lc_index: -1, sw_if_index 10, next index 0, action: 0, match: acl -1 rule -1 trace_bits 00000000 npol_matched_rule_id 25 

  pkt info 0000000000000000 0000000000000000 0000000000000000 0200000bc400000b 000a00061f90de18 0302ffff00000000

   lc_index 0 l3 ip4 11.0.0.196 -> 11.0.0.2 l4 lsb_of_sw_if_index 10 proto 6 l4_is_input 0 l4_slow_path 0 l4_flags 0x00 port 56856 -> 8080 tcp flags (valid) 02 rsvd 0

00:10:28:590256: error-drop

  rx:ipip0

00:10:28:590257: drop

  acl-plugin-out-ip4-fa: ACL deny packets

npol_matched_rule_id 25
Corresponding rule can be checked here:

sh npol interfaces
[tun2 sw_if_index=10  addr=11.0.0.2 addr6=fd20::ae9c:53d8:55c6:a02]
   rx-policy-default:2 rx-profile-default:1 
  tx:
   tx-policy-default:1 tx-profile-default:1 
    [policy#1]
      tx:[rule#0;allow][src==172.18.0.4/32,src==fc00:f853:ccd:e793::4/128,]
    [policy#6]
      tx:[rule#25;deny][proto==TCP,dst==8080,]
      tx:[rule#26;allow][]

sh npol rules
[rule#25;deny][proto==TCP,dst==8080,]

[sknat]

Thanks a bunch, a few comments inline

[sknat]

We can probably avoid the check and always copy the value, that'll be quicker

[hedibouattour]

right

[sknat]

Maybe we could mutate the type a bit more e.g.

typedef struct
{
  u32 next_index;
  u32 sw_if_index;
  fa_full_session_id_t f_sess_id;
  union
  {
    struct
    {
      u32 lc_index;
      u32 match_acl_in_index;
      u32 match_rule_index;
    } acl_info;
    u64[2] caiop_opaque;
  } u64 packet_info[6];
  u32 trace_bitmap;
  u8 fa_flow_state; // fa_flow_state_t
  u8 action;
} acl_fa_trace_t;

where

typedef enum fa_flow_state_t_
{
  FA_TYPE_EXISTING_ACL_FLOW,
  FA_TYPE_NEW_ACL_FLOW,
  FA_TYPE_NEW_CAIOP_FLOW,
} fa_flow_state_t;

Which would allow a few things:

• f_sess_id now being in the trace, we can correlate first and subsequent packets, thus we don't have to keep all the policies metadata in the ACL session table
• depending on the value of fa_flow_state we can print the trace differently (caiop_opaque if FA_TYPE_NEW_CAIOP_FLOW ; acl_info if FA_TYPE_NEW_ACL_FLOW and nothing if FA_TYPE_EXISTING_ACL_FLOW)

[hedibouattour]

Actually looking again at the code, match_rule_index already contains the session id, so no need for an additional field I think.
The reason I didn't follow this approach is I wanted to be able to have the policies for subsequent packets when we don't necessarily have the first, i.e in the middle of a flow. I don't believe we can if we rely on this correlation, can we ?

[sknat]

And would try avoiding this so that we don't fiddle with acl_sessions too much

[hedibouattour]

Any drawback if we use reserved fields without cache-line impact ?

[sknat]

And here, we might want to

• remove trace_bitmap (not sure what it is useful for anyway)
• pass &caiop_opaque

[sknat]

Here npol_match_func can then receive *caiop_opaque which it can cast to

typedef struct
{
  u32 policy_id; // policy id we matched, otherwise ~0
  u32 rule_id; // rule id we matched, otherwise ~0
  int rule_match_res; // stores the code of npol_match_rule
  u32 flag_is_profile:1; // result comes from a profile
  u32 flag_policy_pass:1; // we matched a policy that made us PASS
  u32 flag_pad:30; //
} npol_trace_t;

And then the format would just be a call to format_cnat_trace()

[sknat]

Can you try upstreaming npol changes as dedicated patch to gerrit ?
The acl plugin changes can also probably be collapsed in acl-acl-plugin-custom-policies.patch

[hedibouattour]

Actually we maintain npol part of npol integration into ACL changes as a private plugin as well (part of 0003-acl-acl-plugin-custom-policies.patch) so this can't be upstreamed in the current state.

[sknat]

Let's get this in then and get more info in as things go