[ovs-dev] [PATCHv3 2/6] odp-util: Specify export features when serialising flows.

Andy Zhou azhou at nicira.com
Fri May 16 04:33:16 UTC 2014


Thanks for getting to the bottom of the issue.

On Thu, May 15, 2014 at 8:42 PM, Joe Stringer <joestringer at nicira.com> wrote:
> On 16 May 2014 15:38, Andy Zhou <azhou at nicira.com> wrote:
>>
>> On Thu, May 15, 2014 at 8:34 PM, Joe Stringer <joestringer at nicira.com>
>> wrote:
>> > On 16 May 2014 15:11, Andy Zhou <azhou at nicira.com> wrote:
>> >>
>> >> On Wed, May 14, 2014 at 11:57 PM, Joe Stringer <joestringer at nicira.com>
>> >> wrote:
>> >> > Signed-off-by: Joe Stringer <joestringer at 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.



More information about the dev mailing list