[ovs-dev] [PATCH 1/3] ofproto-dpif-upcall: Translate input port as part of upcall translation.

[Daniele Di Proietto]

2016-06-09 17:46 GMT-07:00 Jesse Gross <jesse at 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 <jesse at 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().

Either way this looks good to me.

Acked-by: Daniele Di Proietto <diproiettod at vmware.com>

Thanks


>      upcall->xout_initialized = true;
>
>      if (!upcall->xout.slow) {
> @@ -1489,7 +1498,7 @@ ukey_create_from_upcall(struct upcall *upcall,
> struct flow_wildcards *wc)
>      atomic_read_relaxed(&enable_megaflows, &megaflow);
>      ofpbuf_use_stack(&maskbuf, &maskstub, sizeof maskstub);
>      if (megaflow) {
> -        odp_parms.odp_in_port = ODPP_NONE;
> +        odp_parms.odp_in_port = wc->masks.in_port.odp_port;
>          odp_parms.key_buf = &keybuf;
>
>          odp_flow_key_from_mask(&odp_parms, &maskbuf);
> --
> 2.5.0
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>