> A packet_in message may be sent for one of two reasons.
> 
> 1. As the result of an controller action supplied in a rule.
>    This is executed if a packet matches the match for the rule.
>    The packet_in reason is explicitly part of the action and
>    is thus correct.

for OFPACT_CONTROLLER (NX thing) it's correct.
for OFPACT_OUTPUT (OF standard way), it isn't.

for the latter, reason=OFPR_ACTION is hardcoded in xlate_output_action.
OF1.3 (1.3.3 5.4.) states reason=table-miss for table-miss flow entry
and thus needs the fixup logic in question.

YAMAMOTO Takashi

> 
> 2. As the result of the failure to match a packet against any rule.
>    In this case a rule present in TBL_INTERNAL is used. This rule
>    has the packet_in reason set to OFPR_NO_MATCH.
> 
>    This is the behaviour described in OpenFlow 1.1 and 1.2 when
>    a miss occurs. And to date it is the behaviour provided
>    by Open vSwtich for all OpenFlow versions.
> 
>    When this behaviour is in effect OFPR_NO_MATCH is the desired
>    packet_in reason and by virtue of the TBL_INTERNAL rule described
>    above is always that value.
> 
> So for both cases the packet_in reason is always correct.
> And following this reasoning there seems to be no need to fix it up.
> 
> What there is a need for, which this patch does not address, is for packets
> to be dropped rather than forwarded to OpenFlow1.3 controllers when a miss
> occurs and an alternate behaviour has not been selected using Table Mod. I
> plan to address this in subsequent patches.
> 
> As well as not been necessary, I believe that the logic that this patch
> removes has not been executed because pin->generated_by_table_miss is never
> true without a separate patch that I have proposed, "ofproto-dpip: Set
> generated_by_table_miss for miss rule".  So I do not expect there to be any
> run-time affect of this change.
> 
> This patch does not remove pin->generated_by_table_miss, although it
> is now set but never used, as I plan to use when implementing the
> OpenFlow1.3 drop-on-miss behaviour described above.
> 
> Cc: YAMAMOTO Takashi <yamam...@valinux.co.jp>
> Signed-off-by: Simon Horman <ho...@verge.net.au>
> ---
>  ofproto/connmgr.c | 32 ++++----------------------------
>  1 file changed, 4 insertions(+), 28 deletions(-)
> 
> diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> index 033ab7d..954a0f9 100644
> --- a/ofproto/connmgr.c
> +++ b/ofproto/connmgr.c
> @@ -1452,8 +1452,7 @@ ofconn_send(const struct ofconn *ofconn, struct ofpbuf 
> *msg,
>  ^L
>  /* Sending asynchronous messages. */
>  
> -static void schedule_packet_in(struct ofconn *, struct ofproto_packet_in,
> -                               enum ofp_packet_in_reason wire_reason);
> +static void schedule_packet_in(struct ofconn *, struct ofproto_packet_in);
>  
>  /* Sends an OFPT_PORT_STATUS message with 'opp' and 'reason' to appropriate
>   * controllers managed by 'mgr'.  For messages caused by a controller
> @@ -1526,25 +1525,6 @@ connmgr_send_flow_removed(struct connmgr *mgr,
>      }
>  }
>  
> -/* Normally a send-to-controller action uses reason OFPR_ACTION.  However, in
> - * OpenFlow 1.3 and later, packet_ins generated by a send-to-controller 
> action
> - * in a "table-miss" flow (one with priority 0 and completely wildcarded) are
> - * sent as OFPR_NO_MATCH.  This function returns the reason that should
> - * actually be sent on 'ofconn' for 'pin'. */
> -static enum ofp_packet_in_reason
> -wire_reason(struct ofconn *ofconn, const struct ofproto_packet_in *pin)
> -{
> -    if (pin->generated_by_table_miss && pin->up.reason == OFPR_ACTION) {
> -        enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
> -
> -        if (protocol != OFPUTIL_P_NONE
> -            && ofputil_protocol_to_ofp_version(protocol) >= OFP13_VERSION) {
> -            return OFPR_NO_MATCH;
> -        }
> -    }
> -    return pin->up.reason;
> -}
> -
>  /* Given 'pin', sends an OFPT_PACKET_IN message to each OpenFlow controller 
> as
>   * necessary according to their individual configurations.
>   *
> @@ -1556,11 +1536,9 @@ connmgr_send_packet_in(struct connmgr *mgr,
>      struct ofconn *ofconn;
>  
>      LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
> -        enum ofp_packet_in_reason reason = wire_reason(ofconn, pin);
> -
> -        if (ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, reason)
> +        if (ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, pin->up.reason)
>              && ofconn->controller_id == pin->controller_id) {
> -            schedule_packet_in(ofconn, *pin, reason);
> +            schedule_packet_in(ofconn, *pin);
>          }
>      }
>  }
> @@ -1586,8 +1564,7 @@ do_send_packet_ins(struct ofconn *ofconn, struct list 
> *txq)
>  /* Takes 'pin', composes an OpenFlow packet-in message from it, and passes it
>   * to 'ofconn''s packet scheduler for sending. */
>  static void
> -schedule_packet_in(struct ofconn *ofconn, struct ofproto_packet_in pin,
> -                   enum ofp_packet_in_reason wire_reason)
> +schedule_packet_in(struct ofconn *ofconn, struct ofproto_packet_in pin)
>  {
>      struct connmgr *mgr = ofconn->connmgr;
>      uint16_t controller_max_len;
> @@ -1595,7 +1572,6 @@ schedule_packet_in(struct ofconn *ofconn, struct 
> ofproto_packet_in pin,
>  
>      pin.up.total_len = pin.up.packet_len;
>  
> -    pin.up.reason = wire_reason;
>      if (pin.up.reason == OFPR_ACTION) {
>          controller_max_len = pin.send_len;  /* max_len */
>      } else {
> -- 
> 1.8.5.2
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to