[ovs-dev] [PATCH] Fix packet-in reason for OpenFlow1.3 table-miss flow entries

[Ben Pfaff]

On Fri, Oct 18, 2013 at 01:41:20PM +0900, YAMAMOTO Takashi wrote:
> >> On Thu, Oct 17, 2013 at 02:11:00PM +0900, YAMAMOTO Takashi wrote:
> >>> > On Wed, Oct 16, 2013 at 05:24:36PM +0900, YAMAMOTO Takashi wrote:
> >>> >> As per spec, make packet-in reason for OpenFlow1.3 table-miss flow
> >>> >> entries no_match rather than action.
> >>> >> 
> >>> >> Signed-off-by: YAMAMOTO Takashi <yamamoto at valinux.co.jp>
> >>> > 
> >>> > Thanks!  I really appreciate that you are working on conformance to
> >>> > later OpenFlow specs.
> >>> > 
> >>> > Before I apply this, let me propose a different idea.  I think that your
> >>> > approach is valid and will work, but it seems to me that it relies on
> >>> > the ofproto-provider implementation keeping track of where the packet-in
> >>> > came from.  Another way would be to notice, when we decode the flow_mod
> >>> > that adds the flow to the flow table, that the flow_mod is for a
> >>> > catch-all flow, and then mark any packet_in ofpacts in the flow_mod as
> >>> > ones that should generate table_miss messages.  Then the
> >>> > ofproto-provider would not have to do anything special, beyond properly
> >>> > passing along a value from the ofpact.
> >>> > 
> >>> > What do you think?
> >>> 
> >>> do you mean:
> >>> - add "reason" member to struct ofpact_output (as ofpact_controller)
> >>> - make ofputil_decode_flow_mod fill it
> >>> 
> >>> i have no strong opinion.  if you prefer it, i will try to implement it.
> >> 
> >> Yes, that's what I mean.  I would prefer to try it this way.  If it is
> >> ugly or infeasible, then the code you have already written makes sense.
> > 
> > ok, i'll try.
> 
> i posted another version which implements the way you suggested.
> 
> btw, the use of pin.reason to determine which of max_len (pin.send_len)
> or miss_send_len to use seems broken for NX CONTROLLER action, which
> seems to allow a user to specify a reason.  while i'm ignorant of NX
> spec (is it publically available?), i guess it's necessary to have
> pin.real_reason or something like that.

NX spec?  There is no spec beyond what is in nicira-ext.h.  There's
nothing private we're holding back.

(I'll read the patch and figure out the desired behavior here later, but
I thought I should respond to that part right away.)