On 16-02-26 02:29 AM, Jiri Pirko wrote:
> Fri, Feb 26, 2016 at 12:20:45AM CET, john.fastab...@gmail.com wrote:
>> In the initial implementation the only way to stop a rule from being
>> inserted into the hardware table was via the device feature flag.
>> However this doesn't work well when working on an end host system
>> where packets are expect to hit both the hardware and software
>> datapaths.
>>
>> For example we can imagine a rule that will match an IP address and
>> increment a field. If we install this rule in both hardware and
>> software we may increment the field twice. To date we have only
>> added support for the drop action so we have been able to ignore
>> these cases. But as we extend the action support we will hit this
>> example plus more such cases. Arguably these are not even corner
>> cases in many working systems these cases will be common.
>>
>> To avoid forcing the driver to always abort (i.e. the above example)
>> this patch adds a flag to add a rule in software only. A careful
>> user can use this flag to build software and hardware datapaths
>> that work together. One example we have found particularly useful
>> is to use hardware resources to set the skb->mark on the skb when
>> the match may be expensive to run in software but a mark lookup
>> in a hash table is cheap. The idea here is hardware can do in one
>> lookup what the u32 classifier may need to traverse multiple lists
>> and hash tables to compute. The flag is only passed down on inserts
>> on deletion to avoid stale references in hardware we always try
>> to remove a rule if it exists.
>>
>> The flags field is part of the classifier specific options. Although
>> it is tempting to lift this into the generic structure doing this
>> proves difficult do to how the tc netlink attributes are implemented
>> along with how the dump/change routines are called. There is also
>> precedence for putting seemingly generic pieces in the specific
>> classifier options such as TCA_U32_POLICE, TCA_U32_ACT, etc. So
>> although not ideal I've left FLAGS in the u32 options as well as it
>> simplifies the code greatly and user space has already learned how
>> to manage these bits ala 'tc' tool.
>>
>> Another thing if trying to update a rule we require the flags to
>> be unchanged. This is to force user space, software u32 and
>> the hardware u32 to keep in sync. Thanks to Simon Horman for
>> catching this case.
>>
>> Signed-off-by: John Fastabend <john.r.fastab...@intel.com>
>> ---
>> include/net/pkt_cls.h | 13 +++++++++++--
>> include/uapi/linux/pkt_cls.h | 1 +
>> net/sched/cls_u32.c | 37 +++++++++++++++++++++++++++----------
>> 3 files changed, 39 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
>> index 6096e96..42dc412 100644
>> --- a/include/net/pkt_cls.h
>> +++ b/include/net/pkt_cls.h
>> @@ -392,12 +392,21 @@ struct tc_cls_u32_offload {
>> };
>> };
>>
>> -static inline bool tc_should_offload(struct net_device *dev)
>> +/* tca flags definitions */
>> +#define TCA_CLS_FLAGS_SOFTWARE 1
>
> I'm sorry, the flag name is misleading to me.
> We have by default, both SW and HW.
>
> Now this flag should say: "do not push to HW".
>
> In future, there will be another flag saying: "do not push to SW".
>
> So I suggest what I already suggested before:
>
> TCA_CLS_FLAGS_SKIP_HW for this one and
> TCA_CLS_FLAGS_SKIP_KERNEL for the future one.
>
> Sounds sane?
>
Sounds reasonable I'll change it in the next revision.
