[ovs-dev] [PATCH 4/6] dpif: Meter framework.

Jesse Gross jesse at nicira.com
Wed Aug 6 21:20:26 UTC 2014


On Wed, Aug 6, 2014 at 9:23 AM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>
>> On Aug 5, 2014, at 6:19 PM, Jesse Gross <jesse at nicira.com> wrote:
>>
>>> On Tue, Aug 5, 2014 at 4:38 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>>> diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
>>> index 271a14e..64761b4 100644
>>> --- a/datapath/linux/compat/include/linux/openvswitch.h
>>> +++ b/datapath/linux/compat/include/linux/openvswitch.h
>>> @@ -590,6 +590,8 @@ struct ovs_action_hash {
>>>  * indicate the new packet contents. This could potentially still be
>>>  * %ETH_P_MPLS if the resulting MPLS label stack is not empty.  If there
>>>  * is no MPLS label stack, as determined by ethertype, no action is taken.
>>> + * @OVS_ACTION_ATTR_METER: Run packet through a meter, which may drop the
>>> + * packet, or modify the packet (e.g., change the DSCP field).
>>>  *
>>>  * Only a single header can be set with a single %OVS_ACTION_ATTR_SET.  Not all
>>>  * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
>>> @@ -608,6 +610,7 @@ enum ovs_action_attr {
>>>        OVS_ACTION_ATTR_HASH,         /* struct ovs_action_hash. */
>>>        OVS_ACTION_ATTR_PUSH_MPLS,    /* struct ovs_action_push_mpls. */
>>>        OVS_ACTION_ATTR_POP_MPLS,     /* __be16 ethertype. */
>>> +       OVS_ACTION_ATTR_METER,        /* u32 meter number. */
>>
>> One problem with sharing a single interface file across platforms is
>> that we can't do this unless there is a Linux implementation. We can
>> pick a high number like before and change it later but that only works
>> for tightly coupled interfaces like dpif-netdev. It wouldn't really be
>> OK if you had an implementation on say, Windows but not Linux.
>
> This may be beside the general point here, but I've had a Linux implementation, and I have posted it before, but it has not been reviewed. The current plan is to get the userspace datapath implementation in first, and then have a better look at the Linux kernel datapath implementation.

Yes, I know :) The concern from before was how this fits in with other
Linux rate limiting code, I don't know if we've made further progress
on that.

> On your point, it seems in future we either need to allow for "reserved, but not yet implemented" code points, or require that new features are added at the same time for all datapaths.

I think the concept of reserved code points will probably not go over
all that well in upstream Linux. Adding features everywhere at the
same time is theoretically possible although we haven't really managed
to do that in the past (such as tunneling in the userspace datapath).
A third possible solution is to actually split up the interface file
by platform but Ben is concerned about the performance cost of
translation.

My opinion is that #3 is inevitable at some point but it's a matter of
when the cost/benefit tradeoff makes sense. So far we've managed to
find ways that keep pushing that day off.



More information about the dev mailing list