ofproto-dpif: Actually log errors in facet_check_consistency()

facet_check_consistency() goes to some effort to create informative
error messages, protected by a rate limit, but does not actually
log these messages.

This appears to be an unintended side effect of
4dff90977694784e67e9c08cc72dee28ebc343ae ("ofproto-dpif: Move odp_actions
from subfacet to facet.").  This patch restores the logging behaviour
present prior to the above commit.

This patch also revers a changes made by the same commit to use
ds_put_cstr() instead of ds_put_char() in facet_check_consistency().

Cc: Justin Pettit <jpettit@nicira.com>
Cc: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
odp-util: Export odp_tun_key_from_attr()

Rename tun_key_from_attr() as odp_tun_key_from_attr() and export it.
This is in preparation for calling this function outside of odp-util.c.

Signed-off-by: Simon Horman <horms@verge.net.au>

---

v12
* First post
Add set skb_mark, set_priority and tunnel support to execute_set_action

Add set skb_mark, set_priority and tunnel support to execute_set_action.

The motivation for this is to allow such actions to be honoured
if they are encountered; by the user-space datapath before recirculation;
or by internal processing of actions by ovs-vswitchd before recirculation.
Recirculation will be added by a subsequent patch.

Signed-off-by: Simon Horman <horms@verge.net.au>

---

v12
* Do not unnecessarily rename flow variable in dp_netdev_port_input()
* Revise changelog

v11
* No change

v10
As suggested by Ben Pfaff
- Correct coding style of prototype of tun_key_from_attr
- Remove dubious memset from set_tunnel_action

v9
* Combined the following patches:
  - Add set skb_priority support to execute_set_action
  - Add set skb_mark support to execute_set_action
  - Add set tunnel support to execute_set_action
* As suggested by Jesse Gross
  - Reduce the number of new parameters added to functions,
    in many cases to zero, by using the skb_priority, skb_mark
    and tunnel elements of struct flow.

v7 - v8
* No change

v6
* First post
ofproto-dpif: Use execute_actions to execute controller actions

Now that execute_actions() is available it can be used as a generic
replacement for special-case action execution in
execute_controller_action().

As suggested by Jesse Gross.

Signed-off-by: Simon Horman <horms@verge.net.au>

---

v12
* Rebase

v11
* No change

v10
* As suggested by Ben Pfaff
- use {} with if statement for OVS_ACTION_ATTR_OUTPUT in execute_actions()
- Do not put required side effects in conditions for ovs_assert();

v9
* First post
Add packet recirculation

Recirculation is a technique to allow a frame to re-enter
frame processing. This is intended to be used after actions
have been applied to the frame with modify the frame in
some way that makes it possible for richer processing to occur.

An example is and indeed targeted use case is MPLS. If an MPLS frame has an
mpls_pop action applied with the IPv4 ethernet type then it becomes
possible to decode the IPv4 portion of the frame. This may be used to
construct a facet that modifies the IPv4 portion of the frame. This is not
possible prior to the mpls_pop action as the contents of the frame after
the MPLS stack is not known to be IPv4.

Design:
* New recirculation action.

  ovs-vswitchd adds a recirculation action to the end of a list of
  datapath actions for a flow when the actions are truncated because
  insufficient flow match information is available to add the next
  OpenFlow action.  The recirculation action is preceded by an action
  to set the skb_mark to an id which can be used to scope a facet lookup
  of a recirculated packet.

  e.g.  pop_mpls(0x0800),dec_ttl becomes pop_mpls(0x800),set(skb_mark(id)),recirculate

* Datapath behaviour

  Then the datapath encounters a recirculate action it:
  + Recalculates the flow key based on the packet
    which will typically have been modified by previous actions
  + As the recirculate action is preceded by a set(skb_mark(id)) action,
    the new match key will now include skb_mark=id.
  + Performs a lookup using the new match key
  + Processes the packet if a facet matches the key or;
  + Makes an upcall if necessary

* No facet behaviour

  + Loop:
    1) translate actions
    2) If there is a recirculate action, execute packet
       and go back to 1) for remaining actions.

Co-Authored-by: Joe Stringer <joe@wand.net.nz>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

Change Log:

v12
* Rebase
* Copy flow tunnel when recirculating in xlate_and_recirculate().
  This avoids a bug that occurred when recirculating more than once
  and thus using flow_storage as the flow passed to flow_extract()
  and flow_storage.tunnel as the tunnel passed to flow_extract().
  The tunnel parameter passed to flow_extract() must not be
  the tunnel element of its flow parameter.
* In xlate_and_recirculate() ensure that there is sufficient headroom in
  packet for both the key, which is present if xlate_and_recirculate()
  is called by handle_flow_miss_without_facet(), and an MPLS LSE in case
  mpls_push() occurs.

v11
* As suggested by Jarno Rajahalem
  - Rename xlate_with_recirculate() as xlate_and_recirculate and add
    a comment above it to make it clearer that it may modify its
    'packet' argument.
  - Only hash in facet_find_by_id() if it is needed.
  - Replace may_recirculate with may_xlate_l3_actions in do_xlate_actions()
    as this seems to be a more intuitive name
  - Fix spelling of execute_recirculate_action()
  - Remove spurious comment regarding setting of the recirculation_id element
    of struct flow in action_xlate_ctx_init() - there is no such element
    at this time
* Enhance comment for recirculation_id element of struct action_xlate_ctx
  to describe valid initial values.
* Enhance comments regarding the use of recirculation ids in facet_revalidate()
* Correct comment for recirculated element of struct action_xlate_ctx,
  previously its described states were the opposite of the implementation.
* Add comments above peek_recirculation_id() and get_recirculation_id()
* Correct error in parameter passed to odp_execute_actions()
  in xlate_and_recirculate()

v10
* Rebase
* Remove execute_actions_for_recircualtion() helper and
  use odp_execute_actions directly()

v9
* Rebase

v8
* For a learn action recirculate regardless of the value of
  ctx->may_learn in do_xlate_actions(). Previously recirculation
  only occurred if ctx->may_learn was true. That is a bug as
  it causes revalidation to fail. Flagged by the tests developed
  as part of a patch to allow multiple MPLS push and poop actions.
* Update tests removing unnecessary dl_type pre-requisite

v7
* Rebase
* Add skb_priority, skb_mark and tun_key parameters to dp_netdev_port_input()
  This allows the skb_priority, skb_mark and tun_key  to be passed from
  dpif_netdev_execute() to honour corresponding set actions.
* Recirculate on reg load and move, stack push and pop,
  multipath, bundle, and learn actions.
* Add tests

v6
* As suggested by Jesse Gross
  - Enforce MAX_RECIRCULATION_DEPTH rather than MAX_RECIRCULATION_DEPTH + 1
    by pre-decrementing rather than post-decrementing loop counter.
  - Stop processing actions when a recirculation action is met
    in validate_and_copy_actions__(). This reflects how
    actions are actually executed and will invalidate actions
    if anything follows recirculation.
  - Add a rate-limited warning for packets dropped because they
    are recirculated too much in ovs_dp_process_received_packet().
  - Pass skb_mark to dp_netdev_sample() rather than using a
    private one.
  - Handle skb_priority when executing recirculation without
    facets in ovs-vswitchd
  - Handle tunnel key when executing recirculation without
    facets in ovs-vswitchd
  - Do not duplicate counting of flow statistics for recirculated packets.
  - Handle revalidation of facets of revalidated packets
    - Also, add facet_lookup_valid_by_id() and use it when
      looking up the parent facet of a recirculated packet instead
      of using the facet without regard for weather it is valid or not.
* Merge with patch "Allow recirculation without facets"
* Merge with patch "Avoid recirculation id collision"

v5
* Correct declaration of facet_find_by_id to match definition:
  ovs_be32 -> uint32_t.

* Enhancements to recirculation id code:
  - Allow efficient lookup of facets by their recirculation id
  - Add RECIRCULATION_ID_DUMMY which may be used in cases
    where no facet it used. It is an arbitrary valid id.
  - Also add recirculated element to action_xlate_ctx()
    to use to detect if a recirculation action was added during
    translation. The previous scheme of checking if recirculation_id
    was not RECIRCULATION_ID_NONE is broken for cases where
    the context is initialised with a recirculation_id other than
    RECIRCULATION_ID_NONE. E.g. when RECIRCULATION_ID_DUMMY is used.

* Avoid recirculation id collision

  Avoid recirculation id collision by checking that an id is
  not already associated with a facet.

  Consecutive recirculation ids are used and thus it is possible for
  there to be situations where a very large number of ids have to
  be checked before finding one that is not already associated with a facet.

  To mitigate the performance impact of such situations a limit on
  the number of checks is in place and if no unused recirculation id
  can be found then the miss is handled without facets as this can
  be done using a dummy recirculation id.

rfc4
* Minor enhancement to recirculation id management: Add RECIRCULATE_ID_NONE
  to use instead of using 0 directly.

* Correct calculation of facet->recirculation_ofpacts and
  facet->recirculation_ofpacts_len in subfacet_make_actions()
  in the case of more than one level of recirculation.

* Allow recirculation without facets

  This covers the following cases:
  - Handle flow miss without facet
    + Previously the use of facets was forced if there was
      any chance of a recirculation action. That is, for
      all flows misses of MPLS packets.
  - Packet Out

rfc3

* Use IS_ERR_OR_NULL()

* Handle facet consistency checking by constructing a chain of facets
  from the given facet, to its recirculation parent and then its parent
  until the topmost facet.  If there is no recirculation  the chain will
  be of length one. If there is one recirculation action then the chain
  will be of length two. And so on.

  The topmost facet in the chain can is used to lookup the rule to be
  verified. The chain is then walked from top to bottom, translating
  actions up to the end or the first recirculation action that is
  encountered, whichever comes first. As the code walks down the chain
  it updates the actions that are executed to start of the actions to
  be executed to be just after the end of the actions executed in the
  previous facet in the chain. This is similar to the way that facets
  are created when a recirculation action is encountered.

rfc2
* As suggested by Jesse Gross
  - Update for changes to ovs_dp_process_received_packet()
    to no longer check if OVS_CB(skb)->flow is pre-initialised.
  - Do not add spurious printk debugging to ovs_execute_actions()
  - Do not add spurious debugging messages to commit_set_nw_action()
  - Correct typo in comment above commit_odp_actions().
  - Do not execute recirculation in ovs-vswitchd, rather allow
    the datapath to make an upcall when a recirculation action
    is encountered on execute.
    + This implicitly breaks support for recirculation without facets,
      so for now force all misses of MPLS frames to be handled with
      a facet; and treat handling of recirculation for packet_out as
      a todo item.
  - Use skb_mark for recirculation_id in match. This avoids
    both expanding the match and including a recirculation_id parameter
    with the recirculation action: set_skb_mark should be used before
    the recirculation action.
  - Tidy up ownership of skb in ovs_execute_actions

rfc1
* Initial post

wip: packet underrun fix
Always update ethertype on mpls_pop

The ethertype should always be updated on mpls_pop
as there may be a transition between MPLS unicast (0x8847) and
MPLS multicast (0x8848).

Ben Pfaff tells me that this is consistent with the
behaviour described in EXT-194 of the JIRA bug tracker.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

---

v12
* Rebse

v11
* Include in recirculation patch-set

v3 - v10
* Not posted

v2
* Add note about EXT-194 to changelog
* Update manpage. Note that I plan to post a patch to
  make use of recirculation to remove restrictions on
  the use of MPLS pop and push.
Allow multiple levels of MPLS push and pop

Also:
* Remove mpls_depth from struct flow,
  This value really has not place in the flow which otherwise
  only knows about the top-most LSE.
  Instead track depth changes during execution.
* Allow MPLS pop to MPLS. That is, popping the top-most LSE of
  an MPLS stack of depth greater than one.

This patch relies on recirculation to update packets to allow
combinations of push and pop actions using a flow which only
knows about the top-most LSE.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

--

v12
* Ensure pre_push_mpls_lse is never used uninitialised in do_xlate_actions()

v11
* Include with recirculation patch-set

* Call peek_recirculation_id() in flow_miss_should_make_facet()
  regardless of dl_type as recirculation can occur on non-MPLS
  packets if there is more than one push_mpls action.
  As observed by Joe Stringer

v2 - v10
* Not posted

fix: pre_push_mpls_lse initialisation
Add MPLS recirculation tests

This patch introduces a python script to generate about 1500 tests for
permutations of mpls_push,mpls_pop,dec_mpls_ttl,dec_ttl where
recirculation occurs up to (and including) three times.

Signed-off-by: Joe Stringer <joe@wand.net.nz>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

v12
* First post
Always use valid ids pointer in dec_ttl_cnt_ids_from_openflow()

Always update the ids pointer after calling ofpbuf_put()
to ensure that it is valid when accessed.

During testing a case came up where the call to ofpbuf_put() in the
for (i = 0; i < ids->n_controllers; i++) loop would cause the underlying
buffer to be reallocated. This resulted in ids->n_controllers being an
incorrect value, the loop continuing on longer than desired and finally a
segmentation fault.

Reported-by: Joe Stringer <joe@wand.net.nz>
Signed-off-by: Simon Horman <horms@verge.net.au>