From: Ben Pfaff Date: Mon, 7 May 2012 19:30:54 +0000 (-0700) Subject: ofp-util: Treat a packet-out in_port of OFPP_CONTROLLER as OFPP_NONE. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=751c778501b0e0726cc0c715f3ad2628c19ba0bb;p=openvswitch ofp-util: Treat a packet-out in_port of OFPP_CONTROLLER as OFPP_NONE. Some OpenFlow 1.0 controllers incorrectly use OPFP_CONTROLLER as the in_port in packet-out messages, when OFPP_NONE is their intent. Until now, Open vSwitch has rejected such requests with an error message. This commit makes Open vSwitch instead treat OFPP_CONTROLLER the same as OFPP_NONE for compatibility with those controllers. (Also, as of this writing, OpenFlow 1.0.1 appears to be changing the port to use from OFPP_NONE to OFPP_CONTROLLER.) Suggested-by: Rob Sherwood Signed-off-by: Ben Pfaff --- diff --git a/lib/odp-util.c b/lib/odp-util.c index 36ea5a4c..fef42ba7 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -1170,7 +1170,7 @@ odp_flow_key_from_flow(struct ofpbuf *buf, const struct flow *flow) nl_msg_put_be64(buf, OVS_KEY_ATTR_TUN_ID, flow->tun_id); } - if (flow->in_port != OFPP_NONE) { + if (flow->in_port != OFPP_NONE && flow->in_port != OFPP_CONTROLLER) { nl_msg_put_u32(buf, OVS_KEY_ATTR_IN_PORT, ofp_port_to_odp_port(flow->in_port)); } diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 0fadadd4..14006f9e 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -2224,7 +2224,7 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po, po->buffer_id = ntohl(opo->buffer_id); po->in_port = ntohs(opo->in_port); if (po->in_port >= OFPP_MAX && po->in_port != OFPP_LOCAL - && po->in_port != OFPP_NONE) { + && po->in_port != OFPP_NONE && po->in_port != OFPP_CONTROLLER) { VLOG_WARN_RL(&bad_ofmsg_rl, "packet-out has bad input port %#"PRIx16, po->in_port); return OFPERR_NXBRC_BAD_IN_PORT; diff --git a/lib/ofp-util.h b/lib/ofp-util.h index bf2ae07b..2e3fb3a4 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -314,12 +314,15 @@ const char *ofputil_packet_in_reason_to_string(enum ofp_packet_in_reason); bool ofputil_packet_in_reason_from_string(const char *, enum ofp_packet_in_reason *); -/* Abstract packet-out message. */ +/* Abstract packet-out message. + * + * ofputil_decode_packet_out() will ensure that 'in_port' is a physical port + * (OFPP_MAX or less) or one of OFPP_LOCAL, OFPP_NONE, or OFPP_CONTROLLER. */ struct ofputil_packet_out { const void *packet; /* Packet data, if buffer_id == UINT32_MAX. */ size_t packet_len; /* Length of packet data in bytes. */ uint32_t buffer_id; /* Buffer id or UINT32_MAX if no buffer. */ - uint16_t in_port; /* Packet's input port or OFPP_NONE. */ + uint16_t in_port; /* Packet's input port. */ union ofp_action *actions; /* Actions. */ size_t n_actions; /* Number of elements in 'actions' array. */ }; diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index c28a95fc..a74bf9ad 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -915,8 +915,27 @@ struct ofproto_class { * * 'flow' reflects the flow information for 'packet'. All of the * information in 'flow' is extracted from 'packet', except for - * flow->in_port, which is taken from the OFPT_PACKET_OUT message. - * flow->tun_id and its register values are zeroed. + * flow->in_port (see below). flow->tun_id and its register values are + * zeroed. + * + * flow->in_port comes from the OpenFlow OFPT_PACKET_OUT message. The + * implementation should reject invalid flow->in_port values by returning + * OFPERR_NXBRC_BAD_IN_PORT. For consistency, the implementation should + * consider valid for flow->in_port any value that could possibly be seen + * in a packet that it passes to connmgr_send_packet_in(). Ideally, even + * an implementation that never generates packet-ins (e.g. due to hardware + * limitations) should still allow flow->in_port values for every possible + * physical port and OFPP_LOCAL. The only virtual ports (those above + * OFPP_MAX) that the caller will ever pass in as flow->in_port, other than + * OFPP_LOCAL, are OFPP_NONE and OFPP_CONTROLLER. The implementation + * should allow both of these, treating each of them as packets generated + * by the controller as opposed to packets originating from some switch + * port. + * + * (Ordinarily the only effect of flow->in_port is on output actions that + * involve the input port, such as actions that output to OFPP_IN_PORT, + * OFPP_FLOOD, or OFPP_ALL. flow->in_port can also affect Nicira extension + * "resubmit" actions.) * * 'packet' is not matched against the OpenFlow flow table, so its * statistics should not be included in OpenFlow flow statistics. diff --git a/tests/ofproto.at b/tests/ofproto.at index 82f2f9c6..89264275 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -529,3 +529,36 @@ check_async 7 OFPR_ACTION OFPPR_ADD ovs-appctl -t ovs-ofctl exit OVS_VSWITCHD_STOP AT_CLEANUP + +dnl This test checks that OFPT_PACKET_OUT accepts both OFPP_NONE (as +dnl specified by OpenFlow 1.0) and OFPP_CONTROLLER (used by some +dnl controllers despite the spec) as meaning a packet that was generated +dnl by the controller. +AT_SETUP([ofproto - packet-out from controller]) +OVS_VSWITCHD_START + +# Start a monitor listening for packet-ins. +AT_CHECK([ovs-ofctl -P openflow10 monitor br0 --detach --no-chdir --pidfile]) +ovs-appctl -t ovs-ofctl ofctl/send 0109000c0123456700000080 +ovs-appctl -t ovs-ofctl ofctl/barrier +ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log +AT_CAPTURE_FILE([monitor.log]) + +# Send some packet-outs with OFPP_NONE and OFPP_CONTROLLER (65533) as in_port. +AT_CHECK([ovs-ofctl packet-out br0 none controller '0001020304050010203040501234']) +AT_CHECK([ovs-ofctl packet-out br0 65533 controller '0001020304050010203040505678']) + +# Stop the monitor and check its output. +ovs-appctl -t ovs-ofctl ofctl/barrier +ovs-appctl -t ovs-ofctl exit + +AT_CHECK([sed 's/ (xid=0x[[0-9a-fA-F]]*)//' monitor.log], [0], [dnl +OFPT_PACKET_IN: total_len=14 in_port=NONE (via action) data_len=14 (unbuffered) +priority:0,tunnel:0,in_port:0000,tci(0) mac(00:10:20:30:40:50->00:01:02:03:04:05) type:1234 proto:0 tos:0 ttl:0 ip(0.0.0.0->0.0.0.0) +OFPT_PACKET_IN: total_len=14 in_port=CONTROLLER (via action) data_len=14 (unbuffered) +priority:0,tunnel:0,in_port:0000,tci(0) mac(00:10:20:30:40:50->00:01:02:03:04:05) type:5678 proto:0 tos:0 ttl:0 ip(0.0.0.0->0.0.0.0) +OFPT_BARRIER_REPLY: +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP