Re: [RFC] net/sched/em_canid: Ematch rule to match CAN frames according to their CAN IDs

[Oliver Hartkopp <socketcan@hartkopp.net>]

Hello Rostislav, hello Eric,

On 12.06.2012 16:50, Rostislav Lisovy wrote:

> On Tue, 2012-06-12 at 16:33 +0200, Eric Dumazet wrote: 
>> On Tue, 2012-06-12 at 16:03 +0200, Eric Dumazet wrote:
>>> On Tue, 2012-06-12 at 15:48 +0200, Rostislav Lisovy wrote:
>>>> em_canid is an ematch capable of classifying CAN frames according to
>>>> their CAN IDs.
>>>>
>>>> This RFC/Patch contains a reworked classifier initially posted in
>>>> http://www.spinics.net/lists/netdev/msg200114.html
>>>> The functionality is the same however there is almost 50% reduction
>>>> in the source code length.
>>>>
>>>> There is a slight difference between this ematch and other available
>>>> ematches. Other ematches implement only a simple match operation and
>>>> are meant to be combined with logic conjunctions (e.g. AND, OR).
>>>> Our ematch makes it possible to use up to 32 rules in single
>>>> 'configuration statement' (with OR semantics). This allows us to take
>>>> the advantage of the bit field data-structure in the implementation of
>>>> the match function.
>>>>
>>>> Example: canid(sff 0x123 eff 0x124 sff 0x230:0x7f0)
>>>> This ematch would match CAN SFF frames with the following IDs:
>>>> 0x123, 0x230--0x23f or EFF frame with ID 0x124.



i tried to figure out the difference between the implementation as classifier
(link above) and this implementation as an ematch.

Do we have any disadvantages using the ematch? E.g. is it still possible to
add additional ematches like checking for patterns inside can_frame.data[]
(which is located in skb->data) with ematch_u32 or e.g. ematch_text ??


...

Btw.

>>>> +struct canid_match {
>>>> +	struct can_filter rules_raw[EM_CAN_RULES_SIZE]; /* Raw rules copied
>>>> +		from netlink message; Used for sending information to
>>>> +		userspace (when 'tc filter show' is invoked) AND when
>>>> +		matching EFF frames*/
>>>> +	DECLARE_BITMAP(match_sff, (1 << CAN_SFF_ID_BITS)); /* For each SFF CAN
>>>> +		ID (11 bit) there is one record in this	bitfield */


these comments have a styling problem :-)

Additional to these checkpatch warnings:

WARNING: line over 80 characters
#144: FILE: net/sched/em_canid.c:48:
+static void em_canid_sff_match_add(struct canid_match *cm, u32 can_id, u32 can_mask)

WARNING: line over 80 characters
#231: FILE: net/sched/em_canid.c:135:
+	if (cm->rules_count > EM_CAN_RULES_SIZE) /* Be sure to fit into the array */

WARNING: line over 80 characters
#256: FILE: net/sched/em_canid.c:160:
+			memcpy(cm->rules_raw + cm->eff_rules_count + cm->sff_rules_count,



Tnx & regards,
Oliver