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

Daniele Di Proietto diproiettod at ovn.org
Mon Jun 13 21:58:07 UTC 2016


2016-06-13 13:47 GMT-07:00 Jesse Gross <jesse at kernel.org>:

> On Mon, Jun 13, 2016 at 1:16 PM, Daniele Di Proietto
> <diproiettod at ovn.org> wrote:
> >
> > 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().
>
> 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.
>
>
Makes sense, thanks



> > Acked-by: Daniele Di Proietto <diproiettod at vmware.com>
>
> Thanks, I'll this series to master and the first patch to branch-2.5
> in a minute.
>



More information about the dev mailing list