Thanks for getting to the bottom of the issue.
On Thu, May 15, 2014 at 8:42 PM, Joe Stringer <joestrin...@nicira.com> wrote:
> 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
