On 16 May 2014 15:38, Andy Zhou <az...@nicira.com> wrote:
> On Thu, May 15, 2014 at 8:34 PM, Joe Stringer <joestrin...@nicira.com>
> wrote:
> > On 16 May 2014 15:11, Andy Zhou <az...@nicira.com> wrote:
> >>
> >> On Wed, May 14, 2014 at 11:57 PM, Joe Stringer <joestrin...@nicira.com>
> >> wrote:
> >> > Signed-off-by: Joe Stringer <joestrin...@nicira.com>
> >> > ---
> >> > v3: First post.
> >> > ---
> >> > lib/odp-util.c | 41 +++++++++++++++++++++++++----------------
> >> > 1 file changed, 25 insertions(+), 16 deletions(-)
> >> >
> >> > diff --git a/lib/odp-util.c b/lib/odp-util.c
> >> > index 6cff2f1..ff3cb7e 100644
> >> > --- a/lib/odp-util.c
> >> > +++ b/lib/odp-util.c
> >> > @@ -2482,16 +2482,16 @@ ovs_to_odp_frag_mask(uint8_t nw_frag_mask)
> >> >
> >> > static void
> >> > odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *flow,
> >> > - const struct flow *mask, odp_port_t
> >> > odp_in_port,
> >> > - size_t max_mpls_depth, bool export_mask)
> >> > + const struct flow *mask, const struct flow
> >> > *data,
> >> > + odp_port_t odp_in_port, size_t
> max_mpls_depth,
> >> > + uint64_t export_mask)
> >> Changing export_mask from bool to uint64_t seems to be overkill. I
> >> hope we can avoid doing this.
> >
> >
> > I realise I didn't explain how this should work, but it was an attempt at
> > simplifying this code (if perhaps a little misguided). Rather than
> passing
> > several parameters to determine special cases for the function, it would
> be
> > nice to be able to specify separately what to serialise and what to not.
> >
> >
> >> How about we the following prototype:
> >> odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *flow,
> >> const struct flow *mask, odp_port_t
> odp_in_port,
> >> size_t max_mpls_depth, bool dp_recirc, bool
> >> export_mask)
> >> --------------------------------------------------------->
> >> ^^^^^^^^^^^^^^^
> >>
> >> /* dp_recirc indicates that datapath understands recirculation */
> >>
> >> The decision of exporting recirc_id and dp_hash will be simple:
> >>
> >> if (dp_recric) {
> >> nl_msg_put_u32(buf, OVS_KEY_ATTR_RECIRC_ID ...)
> >> nl_msg_put_u32(buf, OVS_KEY_ATTR_DP_HASH ... )
> >> }
> >>
> >> We can obtain dp_recirc value similar to max_mpls_depth.
> >>
> >> For example: in ofproto-dpif-upcall.c we can call
> >> ofproto_dpif_get_enable_recirc() to obtain the value.
> >>
> >> dp_recirc can always be set to true within the user space datapath.
> >>
> >> Would this work?
> >
> >
> > I think this would work. We already store the value for dp_recirc, which
> can
> > be retrieved by ofproto_dpif_get_enable_recirc(). If you're happy with
> that,
> > I can resend.
> Thanks. assuming this works for both kernel and user space datapath.
>
I think your proposal is equivalent in behaviour to this patch, which I
tested against an older datapath in the basic case. I'll plan to test that
for the new version.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
