> 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