On Fri, Feb 26, 2016 at 02:29:28PM -0800, Jean Tourrilhes wrote:
> On Fri, Feb 26, 2016 at 12:38:05PM -0800, Ben Pfaff wrote:
> > 
> > I had to look up EXT-427 to find out what we're talking about.  EXT-427
> > extends OFPT_PACKET_OUT in OF1.5 to allow the controller to specify the
> > values of arbitrary metadata fields.  Yes, this would be valuable and
> > I'd be happy to have an implementation.
> 
>       You will need to figure how EXT-427 and NXT_RESUME will play
> together, as both may be used to set metadata.
>       With EXT-427, it's possible to dela with metadata continuation
> entirely from the OXM part of packet-in and packet-out. And because
> OXM is extensible, there is further tricks you can play there.

NXT_RESUME isn't really about saving metadata from packet-in to
packet-out.  If that was all, then OFPAT_SET_FIELD would be sufficient
to restore the metadata.  It's the bits inside NXPINT_CONTINUATION that
are interesting, to preserve state like the "call stack".

Anyway, I found myself wondering why we hadn't already implemented this
part of OF1.5+ OFPT_PACKET_OUT when I wrote the following code in
https://patchwork.ozlabs.org/patch/585521/.  With a proper metadata
abstraction for OFPT_PACKET_OUT everything in the 'for' loop would be
encapsulated inside struct ofputil_packet_out and
ofputil_encode_packet_out():

+    /* Compose actions.
+     *
+     * First, add actions to restore the metadata, then add actions from
+     * 'userdata'.
+     */
+    uint64_t ofpacts_stub[1024 / 8];
+    struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
+    enum ofp_version version = rconn_get_version(swconn);
+
+    for (int id = 0; id < MFF_N_IDS; id++) {
+        const struct mf_field *field = mf_from_id(id);
+
+        if (field->prereqs == MFP_NONE
+            && field->writable
+            && id != MFF_IN_PORT && id != MFF_IN_PORT_OXM
+            && mf_is_set(field, ip_flow))
+        {
+            struct ofpact_set_field *sf = ofpact_put_SET_FIELD(&ofpacts);
+            sf->field = field;
+            sf->flow_has_vlan = false;
+            mf_get_value(field, ip_flow, &sf->value);
+            bitwise_one(&sf->mask, sizeof sf->mask, 0, field->n_bits);
+        }
+    }
+    enum ofperr error = ofpacts_pull_openflow_actions(userdata, userdata->size,
+                                                      version, &ofpacts);
+    if (error) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+        VLOG_WARN_RL(&rl, "failed to parse arp actions (%s)",
+                     ofperr_to_string(error));
+        goto exit;
+    }
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to