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

Andy Zhou azhou at nicira.com
Fri May 16 03:38:11 UTC 2014


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.



More information about the dev mailing list