On Dec 11, 2012, at 1:19 PM, Ben Pfaff <b...@nicira.com> wrote: > On Tue, Dec 11, 2012 at 11:17:33AM -0800, Romain Lenglet wrote: >> On Dec 11, 2012, at 10:47 AM, Ben Pfaff <b...@nicira.com> wrote: >> >>> On Tue, Dec 11, 2012 at 09:54:50AM -0800, Romain Lenglet wrote: >>>> Define NXAST_SAMPLE OpenFlow vendor action and the corresponding >>>> OFPACT_SAMPLE OVS action. >>>> >>>> Signed-off-by: Romain Lenglet <rleng...@vmware.com> >>> >>> I think that this new action is intended to send the packet to a >>> controller when sampling happens, and to allow the packet to "pass >>> through" to additional actions whether it is sampled or not. (The >>> patch doesn't document the intent well enough for me to be certain, >>> and the full implementation isn't here.) >> >> Thanks for your comments. >> Sorry for not providing more context earlier. >> >> The intent is to send an IPFIX packet for every sampled packet. >> It won't be sent to the controller, but to an IPFIX collector. > > OK. That clears some things up. If the action is intended to be > specialized for IPFIX, then ordinarily I would expect it to be named > accordingly.
Good point. I have renamed it into SAMPLE_IPFIX. >> It will be similar to the existing packet sampling implementation, >> but sampling will be done at flow granularity and controlled by the >> controller by inserting such explicit sampling actions. > > That's a reasonable enough approach. > >>> I'm a bit worried about some implementation details. I guess that the >>> restriction to the first action in a flow is to ensure that userspace >>> can supply correct metadata, but I don't think that's sufficient >>> because "resubmit" can cause the first action to be in the middle of >>> the final action set. >> >> The restriction of sampling actions to the beginning of an action >> list is to ensure that the packets sent via IPFIX have not yet been >> modified in the particular flow containing the sampling actions. >> IPFIX supports reporting a packet's headers both before and after >> rewriting, e.g. in the sourceMacAddress and postSourceMacAddress >> fields, and I intend to report only the pre-rewriting headers for >> now. > > OK. Since this is not a technical restriction, but one based on the > intended use, I think that this might be better implemented by > documenting the behavior and the intent, but allowing the user to > place the action wherever he wants, especially since it is possible to > sidestep the restriction through resubmit. I've added that to the action struct's comments in include/openflow/nicira-ext.h. And I've removed the check from the code. >> I think the behaviour with resubmit would be fine. I only want the >> IPFIX packets to reflect the sampled packet's headers before >> modification by any actions in the flow containing the sampling >> action. >> >> I don't understand your comment about "correct metadata." Which >> metadata are you thinking about? > > I mean that kernel flows only have a subset of the information carried > by OpenFlow flows. For example, the kernel does not have the (Nicira > extension to) OpenFlow concept of registers. Userspace has to store > this information somewhere so that it can reconstruct it, if the > packet is to be sent to an OpenFlow controller. > > This may not be an issue for IPFIX since IPFIX presumably does not > have a notion of any of the metadata fields in question either. Right, that's not an issue with IPFIX. Even though IPFIX is extensible, I don't intend to send any non-standard metadata with the packet headers. >> The sampling action cookie contains all the metadata I need. >> To create and send an IPFIX packet I only need: >> - an IPFIX collector's IP address and port, which will be globally >> configured; >> - an IPFIX observation domain ID (unsigned 32-bit), which I will associate >> via the configdb with every datapath; >> - an IPFIX observation point ID (unsigned 32-bit), contained in the sample >> action cookie; >> - the sampled packet headers. >> I don't need any other data. > > OK. Do you plan to add a configuration table for IPFIX, analogous to > the NetFlow or sFlow tables that OVS already has? Yes, that's the plan. Coming in my next patches. Thanks for your comments! _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev