Everything made sense, just a few responses: On Tue, Jan 03, 2012 at 03:46:47PM -0600, Ethan Jackson wrote: > > It might be reasonable to reuse the NXFF_* values for the 'format' > > member in nxt_set_packet_in_format, instead of defining new NXPIF_* > > values. ?Or you could even define a new NXFF_NXM2 that is NXFF_NXM > > plus packet-in support, which would allow you to drop the separate > > NXT_SET_PACKET_IN_FORMAT message. ?But the way you have it is also > > fine, so don't feel obligated to change anything. > > I find this all a bit confusing. I was under the impression that the > FF in NXFF stands for Flow Format. This kind of gets at what I'm > changing, but not perfectly. I suppose using the NXFF values works > until there is no longer a one to one mapping between the flow format > and the packet_in formats. It doesn't seem impossible to me that > something like this could happen which is why I implemented it this > way. At any rate, I don't feel strongly about it either way. I think > the current definition most clear gets at what the feature's intention > is, so I'm slightly inclined to leave it, but I could change it as > well.
Yes, FF stands for flow format. My main take on this patch is that it adopts the NXM flow format for use with packet-in messages, in the same way that the NXM flow format was adopted for flow_mod and flow_removed messages and for flow and aggregate statistics when NXM was originally introduced. But it's not a big deal. Since you don't think it's a good fit, leave it. > > Did you consider adding a struct to encapsulate metadata, e.g. "struct > > flow_metadata" or similar? > > I hadn't considered it. I've tried it just now though and I don't > think it works well. It's conceptually much simpler, but it's fairly > awkward to fit a flow_metadata structure containing the registers, and > flow_metadata into the struct flow because of our reliance on the > FLOW_SIG_SIZE. It is hard to get the padding right. Of course, there > are ways around this which we could utilize in the future. I'm not > sure it's worth it at the moment though. Thoughts? Although it would be ideal to embed the new structure inside struct flow, I actually had in mind to define a structure that wasn't necessarily used inside struct flow. It seems to me that such an approach would better than individual members, even if it falls short of perfection. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
