[ovs-dev] [patch_v1] flow.c: refactor ct_orig_tuple check in miniflow_extract.

[Darrell Ball]

http://openvswitch.org/support/dist-docs/ovs-ofctl.8.pdf


“resubmit([port],[table],ct)
Re-searches this OpenFlow flow table (or the table whose number is specified by table)
with the in_port field replaced by port (if port is specified) and the packet 5-tuple fields
swapped with the corresponding conntrack original direction tuple fields (if ct is speci-
fied, see ct_nw_src above), and executes the actions found, if any, in addition to any
other actions in this flow entry. The in_port and swapped 5-tuple fields are restored
immediately after the search, before any actions are executed.
The ct option requires a valid connection tracking state as a match prerequisite in the flow
where this action is placed. Examples of valid connection tracking state matches include
ct_state=+new, ct_state=+est, ct_state=+rel, and ct_state=+trk-inv.”



On 5/26/17, 8:00 AM, "ovs-dev-bounces at openvswitch.org on behalf of Greg Rose" <ovs-dev-bounces at openvswitch.org on behalf of gvrose8192 at gmail.com> wrote:

    On Fri, 2017-05-26 at 01:53 +0000, Darrell Ball wrote:
    > 
    > On 5/22/17, 4:44 PM, "ovs-dev-bounces at openvswitch.org on behalf of Joe Stringer" <ovs-dev-bounces at openvswitch.org on behalf of joe at ovn.org> wrote:
    > 
    >     On 20 May 2017 at 11:09, Darrell Ball <dlu998 at gmail.com> wrote:
    >     > The checks to populate ct_orig_tuple in miniflow_extract
    >     > includes recirc_id being non-zero.  This is changed here
    >     > to populate the ct_orig_tuple fields based only on ct_state
    >     > per the logic of the design.
    >     >
    >     > Signed-off-by: Darrell Ball <dlu998 at gmail.com>
    >     
    >     I'm not exactly sure what "per the logic of the design" is supposed to
    >     mean, and I suspect that someone reading through the git logs would be
    >     just as lost. As far as I'm aware, any patch to OVS should be "per the
    >     logic of the design", right?
    > 
    > In general, a change can be consistent with an 
    > existing design or a change to the existing design itself. 
    > However, “per the logic of the design” can be assumed by default
    > and hence is usually considered superfluous, which I think is what you
    > want to convey – sure.
    
    At the time I thought you meant more like 'per the specification'.  Now
    I realize I came off as an idiot.  So is there a specification for what
    the ct_orig_tuple fields should be set to?  Is there some rule we can
    refer to that states explicitly that ct_orig_tuple should be populated
    based only upon ct_state?  Or is it something that should just be clear
    by understanding the code?
    
    Sorry if I'm missing something obvious.
    
    - Greg
    
    > 
    > I’ll include the text I used on the related thread, as I agree, the
    > motivation is not necessarily obvious, just by linking ct_orig_tuple to
    > ct_state, as I did in this commit message.
    >     
    >     Do you mean "The ct_orig_tuple fields are designed to be read only if
    >     the ct_state is valid, so do not populate that field if the packet has
    >     not gone through the connection tracker"?
    > 
    >     I think that this kind of
    >     description would provide at least a little extra context to help
    >     understand the motivation behind the change.
    > 
    > 
    > 
    >     _______________________________________________
    >     dev mailing list
    >     dev at openvswitch.org
    >     https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=nAHjQgAdipmU4BHowXyPhoSGZvhBYs0Gprz21Toem1c&s=wrTPUsNoG_MEIRSjPdI7ExOiyaQh-f8tqCJLyyWrlo8&e= 
    >     
    > 
    > 
    > 
    > 
    > 
    > _______________________________________________
    > dev mailing list
    > dev at openvswitch.org
    > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=CX6BNdRBoU5RME8jxR8Ym69dSbg__GatHpbkcUq-nU8&s=hATlW0-8J6fQPfCUJvqQ02qZaTHiCBn6ZjXOtZ26Yqo&e= 
    
    
    
    _______________________________________________
    dev mailing list
    dev at openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=CX6BNdRBoU5RME8jxR8Ym69dSbg__GatHpbkcUq-nU8&s=hATlW0-8J6fQPfCUJvqQ02qZaTHiCBn6ZjXOtZ26Yqo&e=