On Mon, Jun 13, 2016 at 1:16 PM, Daniele Di Proietto
<diproiet...@ovn.org> wrote:
>
> 2016-06-09 17:46 GMT-07:00 Jesse Gross <je...@kernel.org>:
>>
>> When we generate wildcards for upcalled flows, the flows and therefore
>> the wildcards, are in OpenFlow format. These are mostly the same but
>> one exception is the input port. We work around this problem by simply
>> performing an exact match on the input port when generating netlink
>> formatted keys. (This does not lose any information in practice because
>> action translation also always exact matches on input port.)
>>
>> While this works fine for kernel based flows, it misses the userspace
>> datapath, which directly consumes the OFP format mask for the input
>> port. The effect of this is that the in_port mask is sometimes only
>> the lower 16 bits of the field. (This is because OFP format is a 16-bit
>> value stored in a 32-bit field. The full width of the field is initialized
>> with an exact match mask but certain operations result in cleaving this
>> down to 16 bits.) In practice this does not cause a problem because
>> datapath
>> port numbers are almost always in the lower 16 bits of the range anyways.
>>
>> This moves the masking of the datapath format field to translation so that
>> all datapaths see the same result. This also makes more sense conceptually
>> as the input port in the flow is also in ODP format at this stage.
>>
>> Signed-off-by: Jesse Gross <je...@kernel.org>
>> ---
>> ofproto/ofproto-dpif-upcall.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
>> index a18fc5a..9400ef9 100644
>> --- a/ofproto/ofproto-dpif-upcall.c
>> +++ b/ofproto/ofproto-dpif-upcall.c
>> @@ -1083,7 +1083,16 @@ upcall_xlate(struct udpif *udpif, struct upcall
>> *upcall,
>>
>> upcall->dump_seq = seq_read(udpif->dump_seq);
>> upcall->reval_seq = seq_read(udpif->reval_seq);
>> +
>> xlate_actions(&xin, &upcall->xout);
>> + if (wc) {
>> + /* Convert the input port wildcard from OFP to ODP format.
>> There's no
>> + * real way to do this for arbitrary bitmasks since the numbering
>> spaces
>> + * aren't the same. However, flow translation always exact
>> matches the
>> + * whole thing, so we can do the same here. */
>> + WC_MASK_FIELD(wc, in_port.odp_port);
>> + }
>> +
>
>
> I guess the above could be done in xlate_wc_finish().
I did consider that but the problem is that some callers still want
the port to be in OpenFlow format, such as ofproto_trace(). Even
though this is just masking the whole field, I consider the operation
here to be "converting" it to ODP format and so more appropriate for
the upcall code.
The other area where we have this type of split between ODP/OpenFlow
format is with tunnel metadata. That has a flag to indicate which
format it is in which is nice. As part of some larger work I'm
considering making that flag apply to both input port and tunnel
metadata, which might have some benefits for both pieces. If I end up
doing that then I'll move this into the xlate code so that it is
transparent.
> Acked-by: Daniele Di Proietto <diproiet...@vmware.com>
Thanks, I'll this series to master and the first patch to branch-2.5
in a minute.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
