On Thu, Sep 13, 2012 at 09:28:50PM -0700, Ben Pfaff wrote: > On Fri, Sep 14, 2012 at 12:05:44PM +0900, Simon Horman wrote: > > On Thu, Sep 13, 2012 at 01:06:09PM -0700, Ben Pfaff wrote: > > > I get rejects on this against commit 307975da7 currently on master. > > > > > > As long as it needs a rebase anyhow, I see a few more changes I'd > > > like: > > > > > > * The struct ofp12_action_set_field change looks good but the > > > line "- Exactly oxm_len bytes containing a single OXM TLV, > > > then" in the comment above it can now be deleted. > > > > > > * In set_field_format(), "load->dst.ofs" should now be changed > > > to just 0 since we now right-justify the data within the > > > mf_subvalue. > > > > > > * I think that this: > > > > * Accept non-OXM fields in nxm_reg_load_from_openflow12_set_field() > > > didn't quite get done, since I still see the mf->oxm_header > > > == 0 check in nxm_reg_load_from_openflow12_set_field(). > > > > > > * What's the "nast" in set_field_to_nast() stand for? nxast > > > stands for "Nicira eXtension Action <something> Type", but > > > set-field isn't a Nicira extension. > > > > > > * The loop in nxm_reg_load_to_nxast() is very similar to a > > > loop in learn_execute(). Can we factor it out into a helper > > > function? > > > > Hi Ben, > > > > I think that the patch below addresses all of the points above. > > Thanks. > > There wasn't as much common code in the loops as I thought there was. I > switched back to the earlier version.
Thanks, I wasn't particularly attached to the new code. > I discovered that there was still a function with _nast in its name. I > changed it to _nxast. Thanks, sorry for missing that. > With those changes, I applied this to master. Thanks again! Thanks! _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
