[ovs-dev] Sample action for patch port

[Ben Pfaff]

On Tue, Nov 15, 2016 at 05:45:06AM +0000, Daniel Ye wrote:
> As we know, NXAST_RAW_SAMPLE2 was introduced to OVS to make flow-based
> IPFIX to support tunnel information exporting.  A field called
> “sampling_port” is added in NXAST_RAW_SAMPLE2 and it’s an
> ofp-port. This field is not only used for exporting tunnel
> information, but also used for judging the flow direction.
> 
> However, recently I found NXAST_RAW_SAMPLE2 cannot be used for patch
> port, because there is no odp-port for a patch port. The conversion
> between NXAST_RAW_SAMPLE2 and datapath sample action will fail.  The
> simple solution about this problem is that we can use NXAST_RAW_SAMPLE
> openflow action instead of NXAST_RAW_SAMPLE2. But the limitation is
> that NXAST_RAW_SAMPLE action didn’t have a field to imply the flow
> direction.  According to our discuss, a better solution is that adding
> NXAST_RAW_SAMPLE3, which have a direction field to show the flow
> direction. This sample action will looks like sample(…, ingress|
> egress).

Yes, all of this matches what we talked about earlier today.  It should
be easy to add support for "ingress" and "egress" keywords on the sample
action.  It looks like NXAST_RAW_SAMPLE2 was planned well enough that we
don't even need to introduce a NXAST_RAW_SAMPLE3.  That part should be
simple and straightforward.

I started looking through the code to see what is needed.  The part that
I do not understand yet is what should happen differently when one of
these keywords is specified.  For the case where a tunnel port is
specified, the code retrieves information related to tunneling and puts
that into the IPFIX report.  But I haven't noticed, yet, what the sample
action should report differently over IPFIX if it knows that it is an
ingress or egress but it does not have tunnel information.

Actually, looking closer, I guess that there is this code in
ipfix_cache_entry_init() which adds to the IPFIX report whether the
packet is sampled on ingress or egress:

        data_common->flow_direction =
            (output_odp_port == ODPP_NONE) ? INGRESS_FLOW : EGRESS_FLOW;

Is that all that should change in the IPFIX report based on this
keyword?  If so, then I think that this should be really simple to
implement.  In that case, I'm willing to propose a patch myself, if you
would like that.

Thanks,

Ben.