Currently, we assume that OpenFlow 1.3 set_field actions can contain
multiple header match fields (OXMs). According to specification, a
set_field action contains exactly one OXM, followed by padding to align
the action field in a OpenFlow message to 64 bits.
This diff changes how we handle OpenFlow messages that contain sef_field
actions to match the specs.
Feedback? OK?
Ayaka
Index: usr.sbin/switchd/ofp13.c
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/ofp13.c,v
retrieving revision 1.46
diff -u -p -u -r1.46 ofp13.c
--- usr.sbin/switchd/ofp13.c 21 Nov 2019 06:22:57 -0000 1.46
+++ usr.sbin/switchd/ofp13.c 26 Nov 2019 23:01:44 -0000
@@ -780,7 +780,7 @@ ofp13_validate_action(struct switchd *sc
print_map(type, ofp_action_map), len, ant->ant_ttl);
break;
case OFP_ACTION_SET_FIELD:
- if (len < sizeof(*asf))
+ if (len < sizeof(*asf) || len != OFP_ALIGN(len))
return (-1);
if ((asf = ibuf_seek(ibuf, *off, sizeof(*asf))) == NULL)
return (-1);
@@ -791,16 +791,14 @@ ofp13_validate_action(struct switchd *sc
print_map(type, ofp_action_map), len);
len -= sizeof(*asf) - sizeof(asf->asf_field);
- while (len > 0) {
- if ((oxm = ibuf_seek(ibuf, moff, sizeof(*oxm)))
- == NULL)
- return (-1);
- if (ofp13_validate_oxm(sc, oxm, oh, ibuf, moff) == -1)
- return (-1);
+ if ((oxm = ibuf_seek(ibuf, moff, sizeof(*oxm))) == NULL)
+ return (-1);
+ if (ofp13_validate_oxm(sc, oxm, oh, ibuf, moff) == -1)
+ return (-1);
- len -= sizeof(*oxm) + oxm->oxm_length;
- moff += sizeof(*oxm) + oxm->oxm_length;
- }
+ len -= sizeof(*oxm) + oxm->oxm_length;
+ if (len >= OFP_ALIGNMENT)
+ return (-1);
break;
default:
Index: sys/net/switchofp.c
===================================================================
RCS file: /cvs/src/sys/net/switchofp.c,v
retrieving revision 1.75
diff -u -p -u -r1.75 switchofp.c
--- sys/net/switchofp.c 21 Nov 2019 17:24:15 -0000 1.75
+++ sys/net/switchofp.c 26 Nov 2019 23:07:51 -0000
@@ -2174,7 +2174,8 @@ swofp_validate_action(struct switch_soft
}
break;
case OFP_ACTION_SET_FIELD:
- if (ahlen < sizeof(struct ofp_action_set_field)) {
+ if (ahlen < sizeof(struct ofp_action_set_field) ||
+ ahlen != OFP_ALIGN(ahlen)) {
*err = OFP_ERRACTION_LEN;
return (-1);
}
@@ -2189,22 +2190,37 @@ swofp_validate_action(struct switch_soft
dptr = (uint8_t *)ah;
dptr += sizeof(struct ofp_action_set_field) -
offsetof(struct ofp_action_set_field, asf_field);
- while (oxmlen > 0) {
- oxm = (struct ofp_ox_match *)dptr;
- if (swofp_validate_oxm(oxm, err)) {
- if (*err == OFP_ERRMATCH_BAD_LEN)
- *err = OFP_ERRACTION_SET_LEN;
- else
- *err = OFP_ERRACTION_SET_TYPE;
+ oxm = (struct ofp_ox_match *)dptr;
+ oxmlen -= sizeof(struct ofp_ox_match);
+ if (oxmlen < oxm->oxm_length) {
+ *err = OFP_ERRACTION_SET_LEN;
+ return (-1);
+ }
+ /* Remainder is padding. */
+ oxmlen -= oxm->oxm_length;
+ if (oxmlen >= OFP_ALIGNMENT) {
+ *err = OFP_ERRACTION_SET_LEN;
+ return (-1);
+ }
+ if (swofp_validate_oxm(oxm, err)) {
+ if (*err == OFP_ERRMATCH_BAD_LEN)
+ *err = OFP_ERRACTION_SET_LEN;
+ else
+ *err = OFP_ERRACTION_SET_TYPE;
+ return (-1);
+ }
+
+ dptr += sizeof(struct ofp_ox_match) + oxm->oxm_length;
+ while (oxmlen > 0) {
+ if (*dptr != 0) {
+ *err = OFP_ERRACTION_SET_ARGUMENT;
return (-1);
}
-
- dptr += sizeof(*oxm) + oxm->oxm_length;
- oxmlen -= sizeof(*oxm) + oxm->oxm_length;
+ oxmlen--;
+ dptr++;
}
break;
-
default:
/* Unknown/unsupported action. */
*err = OFP_ERRACTION_TYPE;
Index: usr.sbin/tcpdump/print-ofp.c
===================================================================
RCS file: /cvs/src/usr.sbin/tcpdump/print-ofp.c,v
retrieving revision 1.11
diff -u -p -u -r1.11 print-ofp.c
--- usr.sbin/tcpdump/print-ofp.c 28 Nov 2016 17:47:15 -0000 1.11
+++ usr.sbin/tcpdump/print-ofp.c 27 Nov 2019 00:15:12 -0000
@@ -232,10 +232,40 @@ ofp_print_setconfig(const u_char *bp, u_
}
void
+ofp_print_oxm_field(const u_char *bp, u_int length, int omlen, int once)
+{
+ struct ofp_ox_match *oxm;
+
+ parse_next_oxm:
+ if (length < sizeof(*oxm)) {
+ printf(" [|OpenFlow]");
+ return;
+ }
+
+ oxm = (struct ofp_ox_match *)bp;
+ bp += sizeof(*oxm);
+ length -= sizeof(*oxm);
+ if (length < oxm->oxm_length) {
+ printf(" [|OpenFlow]");
+ return;
+ }
+
+ ofp_print_oxm(oxm, bp, length);
+ bp += oxm->oxm_length;
+ length -= oxm->oxm_length;
+
+ if (once)
+ return;
+
+ omlen -= min(sizeof(*oxm) + oxm->oxm_length, omlen);
+ if (omlen)
+ goto parse_next_oxm;
+}
+
+void
ofp_print_packetin(const u_char *bp, u_int length)
{
struct ofp_packet_in *pin;
- struct ofp_ox_match *oxm;
int omtype, omlen;
int haspacket = 0;
const u_char *pktptr;
@@ -276,27 +306,7 @@ ofp_print_packetin(const u_char *bp, u_i
if (omlen == 0)
goto print_packet;
- parse_next_oxm:
- if (length < sizeof(*oxm)) {
- printf(" [|OpenFlow]");
- return;
- }
-
- oxm = (struct ofp_ox_match *)bp;
- bp += sizeof(*oxm);
- length -= sizeof(*oxm);
- if (length < oxm->oxm_length) {
- printf(" [|OpenFlow]");
- return;
- }
-
- ofp_print_oxm(oxm, bp, length);
-
- bp += oxm->oxm_length;
- length -= oxm->oxm_length;
- omlen -= min(sizeof(*oxm) + oxm->oxm_length, omlen);
- if (omlen)
- goto parse_next_oxm;
+ ofp_print_oxm_field(bp, length, omlen, 0);
print_packet:
if (haspacket == 0)
@@ -322,7 +332,6 @@ void
ofp_print_flowremoved(const u_char *bp, u_int length)
{
struct ofp_flow_removed *fr;
- struct ofp_ox_match *oxm;
int omtype, omlen;
if (length < sizeof(*fr)) {
@@ -355,27 +364,7 @@ ofp_print_flowremoved(const u_char *bp,
bp += sizeof(*fr);
length -= sizeof(*fr);
- parse_next_oxm:
- if (length < sizeof(*oxm)) {
- printf(" [|OpenFlow]");
- return;
- }
-
- oxm = (struct ofp_ox_match *)bp;
- bp += sizeof(*oxm);
- length -= sizeof(*oxm);
- if (length < oxm->oxm_length) {
- printf(" [|OpenFlow]");
- return;
- }
-
- ofp_print_oxm(oxm, bp, length);
-
- bp += oxm->oxm_length;
- length -= oxm->oxm_length;
- omlen -= min(sizeof(*oxm) + oxm->oxm_length, omlen);
- if (omlen)
- goto parse_next_oxm;
+ ofp_print_oxm_field(bp, length, omlen, 0);
}
void
@@ -453,7 +442,6 @@ void
ofp_print_flowmod(const u_char *bp, u_int length)
{
struct ofp_flow_mod *fm;
- struct ofp_ox_match *oxm;
struct ofp_instruction *i;
int omtype, omlen, ilen;
int instructionslen, padsize;
@@ -498,27 +486,10 @@ ofp_print_flowmod(const u_char *bp, u_in
goto parse_next_instruction;
}
- parse_next_oxm:
- if (length < sizeof(*oxm)) {
- printf(" [|OpenFlow]");
- return;
- }
-
- oxm = (struct ofp_ox_match *)bp;
- bp += sizeof(*oxm);
- length -= sizeof(*oxm);
- if (length < oxm->oxm_length) {
- printf(" [|OpenFlow]");
- return;
- }
+ ofp_print_oxm_field(bp, length, omlen, 0);
- ofp_print_oxm(oxm, bp, length);
-
- bp += oxm->oxm_length;
- length -= oxm->oxm_length;
- omlen -= min(sizeof(*oxm) + oxm->oxm_length, omlen);
- if (omlen)
- goto parse_next_oxm;
+ bp += omlen;
+ length -= omlen;
/* Skip padding if any. */
if (padsize) {
@@ -1017,7 +988,6 @@ void
action_print_setfield(const u_char *bp, u_int length)
{
struct ofp_action_set_field *asf;
- struct ofp_ox_match *oxm;
int omlen;
if (length < (sizeof(*asf) - AH_UNPADDED)) {
@@ -1030,27 +1000,7 @@ action_print_setfield(const u_char *bp,
if (omlen == 0)
return;
- parse_next_oxm:
- if (length < sizeof(*oxm)) {
- printf(" [|OpenFlow]");
- return;
- }
-
- oxm = (struct ofp_ox_match *)bp;
- bp += sizeof(*oxm);
- length -= sizeof(*oxm);
- if (length < oxm->oxm_length) {
- printf(" [|OpenFlow]");
- return;
- }
-
- ofp_print_oxm(oxm, bp, length);
-
- bp += oxm->oxm_length;
- length -= oxm->oxm_length;
- omlen -= min(sizeof(*oxm) + oxm->oxm_length, omlen);
- if (omlen)
- goto parse_next_oxm;
+ ofp_print_oxm_field(bp, length, omlen, 1);
}
void
@@ -1094,6 +1044,7 @@ ofp_print_action(struct ofp_action_heade
break;
case OFP_ACTION_SET_FIELD:
+ action_print_setfield(bp, length);
break;
case OFP_ACTION_COPY_TTL_OUT:
