On Fri, Jul 11, 2014 at 4:31 PM, Romain Lenglet <rleng...@vmware.com> wrote: > On 7/11/14, 3:53 PM, "Pravin Shelar" <pshe...@nicira.com> wrote: > > >>On Sun, Jun 29, 2014 at 8:41 PM, Wenyu Zhang <wen...@vmware.com> wrote: >>> Extend IPFIX exporter to export tunnel headers when both input and >>>output >>> of the port. >>> Add three other_config options in IPFIX table: enable-input-sampling, >>> enable-output-sampling and enable-tunnel-sampling, to control whether >>> sampling tunnel info, on which direction (input or output). >>> Insert sampling action before output action and the output tunnel port >>> is sent to datapath in the sampling action. >>> Make datapath collect output tunnel info and send it back to userpace >>> in upcall message with a new additional optional attribute. >>> Add a tunnel ports map to make the tunnel port lookup faster in sampling >>> upcalls in IPFIX exporter. Make the IPFIX exporter generate IPFIX >>>template >>> sets with enterprise elements for the tunnel info, save the tunnel info >>> in IPFIX cache entries, and send IPFIX DATA with tunnel info. >>> Add flowDirection element in IPFIX templates. >>> >>> Signed-off-by: Wenyu Zhang <wen...@vmware.com> >>> Acked-by: Romain Lenglet <rleng...@vmware.com> >>> --- >> >>I have not reviewed patch completely but I have high level comment >>about extra route lookup done for sampled packets. >> >>I think we can avoid it if we delay upcall and do it in vport->send() >>when we have all the data. >>Sample action can just flag the skb for sampling and let vport->send >>to actually do it. >>This is also more accurate because it is using same route that tunnel >>send would use compared to get_out_tun_key() which can use older >>skb->mark for route lookup. >>Also we can also avoid code duplication done for get_out_tun_key(). > > IMHO, this would be a more intrusive change. > Our design uses a single mechanism (sample + userspace actions) for both > ingress and egress sampling. Your mechanism would apply only to egress > sampling and would be quite different from ingress sampling. > We would also lose the flexibility of the current sample action, which > supports executing arbitrary actions when a packet is sampled. > I also believe it would be more complicated to refactor the logic of the > userspace action so that it’s callable from within vports. I could be > wrong though. > I would personally prefer to keep the vport implementations decoupled from > specific action logic, such as the userspace action. > > We explicitly chose the tradeoff to have 2 route table lookups. > It keeps the design simpler, with the sampling logic and vport > implementation decoupled. > And the lookup inconsistencies are only temporary, during table changes, > which shouldn’t happen often. > We considered fixing those rare inconsistencies were not worth the > increased complexity.
I agree that ingress is complicate in the other design. Therefore for now we can keep the existing design. I will post further review comments later. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
