Hello,
Recently I tested OpenBSD's OpenFlow support with the FAUCET SDN controller
(https://faucet.nz).
There were some minor issues - OpenFlow spec ambiguity with length checks,
endianness of the cookie in the packet in message, not able to match a
packet with no VLAN, and matches running against a copy of the packet that
did not have pending set field changes between tables applied.
My tests were done with the FAUCET controller test suite. It passes the
basic switching tests, but there are some limitations I may be able to
address in the future (e.g. it supports only a single controller, and there
is no OFPortStatus message support so the controller doesn't know if a port
goes up or down at runtime).
I attach my patch, and a link to a PR for the FAUCET controller which would
allow us to add OpenBSD as a supported platform, which would be nice:
https://github.com/faucetsdn/faucet/pull/3728
I would appreciate your guidance and feedback, and hope to make further
changes in the future.
Thanks,
diff --git a/sys/net/ofp.h b/sys/net/ofp.h
index 8ba6ce1a90d..9d03a3c418e 100644
--- a/sys/net/ofp.h
+++ b/sys/net/ofp.h
@@ -480,6 +480,11 @@ struct ofp_flow_mod {
struct ofp_match fm_match;
} __packed;
+/* OpenFlow 1.3.5 secion 7.2.3.1 is ambiguous on whether length field,
+ includes length of type and length fields. A controller might send
+ either length 4 or length 0 for no matches. */
+#define OFP_MATCH_MIN_LEN 4
+
/* Flow removed reasons */
#define OFP_FLOWREM_REASON_IDLE_TIMEOUT 0 /* Flow idle time exceeded idle_timeout */
#define OFP_FLOWREM_REASON_HARD_TIMEOUT 1 /* Time exceeded hard_timeout */
diff --git a/sys/net/switchofp.c b/sys/net/switchofp.c
index ba7ee0df95f..32a218ed04c 100644
--- a/sys/net/switchofp.c
+++ b/sys/net/switchofp.c
@@ -2867,24 +2867,27 @@ swofp_ox_match_vlan_vid(struct switch_flow_classify *swfcl,
{
uint16_t in, mth, mask = 0;
+ memcpy(&mth, oxm->oxm_value, sizeof(uint16_t));
+
+ /*
+ * OpenFlow Switch Specification ver 1.3.5 says if oxm value
+ * is OFP_XM_VID_NONE, matches only packets without a VLAN tag
+ */
+ if (mth == htons(OFP_XM_VID_NONE)) {
+ if (swfcl->swfcl_vlan == NULL)
+ return (0);
+ }
+
if (swfcl->swfcl_vlan == NULL)
return (1);
- in = swfcl->swfcl_vlan->vlan_vid;
- memcpy(&mth, oxm->oxm_value, sizeof(uint16_t));
-
if (OFP_OXM_GET_HASMASK(oxm))
memcpy(&mask, oxm->oxm_value + sizeof(uint16_t),
sizeof(uint16_t));
else
mask = UINT16_MAX;
- /*
- * OpenFlow Switch Specification ver 1.3.5 says if oxm value
- * is OFP_XM_VID_NONE, matches only packets without a VLAN tag
- */
- if (mth == htons(OFP_XM_VID_NONE))
- return (1);
+ in = swfcl->swfcl_vlan->vlan_vid;
/*
* OpenFlow Switch Specification ver 1.3.5 says if oxm value and mask
@@ -3429,7 +3432,7 @@ swofp_action_output_controller(struct switch_softc *sc, struct mbuf *m0,
pin->pin_buffer_id = htonl(OFP_PKTOUT_NO_BUFFER);
pin->pin_table_id = swpld->swpld_table_id;
- pin->pin_cookie = swpld->swpld_cookie;
+ pin->pin_cookie = htobe64(swpld->swpld_cookie);
pin->pin_reason = reason;
if (frame_max_len) {
@@ -4616,6 +4619,7 @@ swofp_forward_ofs(struct switch_softc *sc, struct switch_flow_classify *swfcl,
TAILQ_FOREACH(swft, &ofs->swofs_table_list, swft_table_next) {
if (swft->swft_table_id != next_table_id)
continue;
+ int table_actions_pending = 0;
/* XXX
* The metadata is pipeline parameters but it uses same match
@@ -4646,6 +4650,7 @@ swofp_forward_ofs(struct switch_softc *sc, struct switch_flow_classify *swfcl,
swfe->swfe_apply_actions);
if (m == NULL)
goto out;
+ table_actions_pending = 1;
}
if (swfe->swfe_clear_actions) {
@@ -4658,6 +4663,7 @@ swofp_forward_ofs(struct switch_softc *sc, struct switch_flow_classify *swfcl,
if (swfe->swfe_write_actions) {
error = swofp_write_actions(
swfe->swfe_write_actions, swpld);
+ table_actions_pending = 1;
if (error)
goto out;
}
@@ -4669,6 +4675,10 @@ swofp_forward_ofs(struct switch_softc *sc, struct switch_flow_classify *swfcl,
next_table_id = swfe->swfe_goto_table->igt_table_id;
else
break;
+
+ if (table_actions_pending)
+ if ((m = swofp_apply_set_field(m, swpld)) == NULL)
+ goto out;
}
m = swofp_execute_action_set(sc, m, swpld);
@@ -5127,7 +5137,7 @@ swofp_flow_mod_cmd_add(struct switch_softc *sc, struct mbuf *m)
* 2) OXM filters can't be bigger than the packet.
*/
if (omlen < sizeof(*om) ||
- omlen >= (m->m_len - sizeof(*ofm))) {
+ omlen > (m->m_len - sizeof(*ofm) + OFP_MATCH_MIN_LEN)) {
etype = OFP_ERRTYPE_BAD_MATCH;
error = OFP_ERRMATCH_BAD_LEN;
goto ofp_error;
@@ -5249,7 +5259,7 @@ swofp_flow_mod_cmd_common_modify(struct switch_softc *sc, struct mbuf *m,
* 2) OXM filters can't be bigger than the packet.
*/
if (omlen < sizeof(*om) ||
- omlen >= (m->m_len - sizeof(*ofm))) {
+ omlen > (m->m_len - sizeof(*ofm) + OFP_MATCH_MIN_LEN)) {
etype = OFP_ERRTYPE_BAD_MATCH;
error = OFP_ERRMATCH_BAD_LEN;
goto ofp_error;
@@ -5339,7 +5349,7 @@ swofp_flow_mod_cmd_common_delete(struct switch_softc *sc, struct mbuf *m,
* 2) OXM filters can't be bigger than the packet.
*/
if (omlen < sizeof(*om) ||
- omlen >= (m->m_len - sizeof(*ofm))) {
+ omlen > (m->m_len - sizeof(*ofm) + OFP_MATCH_MIN_LEN)) {
etype = OFP_ERRTYPE_BAD_MATCH;
error = OFP_ERRMATCH_BAD_LEN;
goto ofp_error;
@@ -5403,7 +5413,7 @@ swofp_flow_mod(struct switch_softc *sc, struct mbuf *m)
ofm = mtod(m, struct ofp_flow_mod *);
ohlen = ntohs(ofm->fm_oh.oh_length);
if (ohlen < sizeof(*ofm) ||
- ohlen < (sizeof(*ofm) + ntohs(ofm->fm_match.om_length))) {
+ ohlen < (sizeof(*ofm) + ntohs(ofm->fm_match.om_length) - OFP_MATCH_MIN_LEN)) {
swofp_send_error(sc, m, OFP_ERRTYPE_BAD_REQUEST,
OFP_ERRREQ_BAD_LEN);
return (-1);
