Hi Joe, Thanks for the response.
Agree with your MPLS points. I was just using the MPLS example as a hypothetical situation as I noticed it called the action commit function so it could be used to modify IP address fields before Conntrack that were not be modified in other ways. I was more looking clarification that the ordering of the rule actions is important in ovs-conntrack so that if we do an IP address set field followed by a CT() then the updated IP field should pass into nf_conntrack rather than it being the original tuples (as is now). To me, the former makes more sense here. For example, if the OVS was implementing some form of NAT along with connection tracking then using the input tuples and not updated fields means we could not track flows bidirectionally. I will take from your response that OVS-Conntrack should consider the ordering in this way? I 'think' the fix for this in the code is quite simple. Just move the action commit call from the compose_recirculate_action__() in ofproto-dpif-xlate.c to before the netlink message generation in compose_conntrack_action() in the same file. That way previous actions will be pushed before the conntrack action. The commit code would also need to be added to compose_recirculation_action() to ensure the actions are committed if a recirculation action occurs outside of conntrack. Thanks, John On Thu, Jun 25, 2015 at 10:05 AM, Joe Stringer <[email protected]> wrote: > Hi John, > > Thanks for reporting this. It sounds like a bug. As an OpenFlow user, > It would be surprising for the order of these actions in the action > list to be rearranged like you report. For many fields this doesn't > matter (eg, if you only write IP then write TCP port), however for > conntrack, the order is more important - when you change the IP > address affects how the connection is tracked. So the flow generation > for the first rule looks wrong from that perspective. > > The core problem here is that while processing the actions list, many > of the "set_field" operations are deferred until the end of the > processing, whereas conntrack action is not. I think we need to come > up with some way to ensure that the flow changes are committed before > adding the conntrack action to the datapath actions list. I suspect > much of the code is already there for this, it just needs to be hooked > up correctly. > > For the second flow, in some ways I'm inclined to go with the same > approach - Commit the MPLS before committing the conntrack action. > Also, I don't know what you're expecting the connection tracker to do > when it receives MPLS packets. I doubt that the connection tracker > would understand how to track MPLS packets or associate them with > connections. I'm not sure what behaviour you're getting today, but I > would expect all such packets to come back with conn_state=+trk+inv > (ie, invalid). What does it even mean to connection-track MPLS > packets? > > On 24 June 2015 at 18:12, John Hurley <[email protected]> wrote: > > Hi, > > > > I have been playing about with ovs-conntrack and noticed an issue that > could > > be a bug. Either that or my understanding is incorrect and would > appreciate > > clarification. > > > > When we add a rule with a ct(recirc) action I notice that the call to > > Conntrack is always the first action in the kernel rule that is created. > In > > ofproto-dpif-xlate.c a call to compose_conntrack_action() will append a > > nl_msg for this action, then call recirculate which uses > > commit_odp_actions() to add messages for previous actions before adding > its > > own recirc action to kernel rule action list. However, I have also > noticed > > that when a push mpls action is added that this commit_odp_actions() > > function is also called which can affect the tuples sent to nf_conntrack. > > > > e.g. adding the rule: > > ovs-ofctl add-flow br0 > > "conn_state=-trk,action=set_field:1.2.3.4->nw_dst,ct(recirc)" > > > > will result in a kernel rule with actions of order 'ct, set dst ip, > recirc' > > and the sk_buff sent to nf_conntrack_in will have the source and > destination > > IP addresses of the matching packet. > > > > > > adding the rule: > > ovs-ofctl add-flow br0 > > > "conn_state=-trk,action=set_field:1.2.3.4->nw_dst,push_mpls:0x8847,ct(recirc)" > > > > will have actions ordered 'set dst ip, ct, push mpls, recirc' and the > > destination IP sent into nf_conntrack will be 1.2.3.4 > > > > Can you clarify if this discrepancy is a bug? Also, can you clarify what > is > > the correct way ovs should support Conntrack in an action list? For > example, > > in the first rule, is it correct that Conntrack should be applied on the > > input packet tuples or should the set field be applied before Conntrack > as > > it appears before it in the ovs-ofctl rule? > > > > Thanks, > > John > > > > > > > > _______________________________________________ > > discuss mailing list > > [email protected] > > http://openvswitch.org/mailman/listinfo/discuss > > >
_______________________________________________ discuss mailing list [email protected] http://openvswitch.org/mailman/listinfo/discuss
