Re: [PATCH RFC net-next v2] net: dsa: mv88e6xxx: Add partial support for TCAM entries

[Cedric Jehasse]

Hi,

>> +#define mv88e6xxx_tcam_match_set(key, _offset, data, mask) \
>> +     do { \
>> +             typeof(_offset) (offset) = (_offset); \
>> +             BUILD_BUG_ON((offset) + sizeof((data)) > TCAM_MATCH_SIZE); \
>> +             __mv88e6xxx_tcam_match_set(key, offset, sizeof(data), \
>> +                                        (u8 *)&(data), (u8 *)&(mask)); \
>> +     } while (0)
>> +
>> +static inline void __mv88e6xxx_tcam_match_set(struct mv88e6xxx_tcam_key *key, unsigned int offset,
>> +                                           size_t size, u8 *data, u8 *mask)
>> +{
>> +     memcpy(&key->frame_data[offset], data, size);
>> +     memcpy(&key->frame_mask[offset], mask, size);
>> +}
>
>I want to look at these two in more detail, i don't like macros like
>this, nor inline functions in header files. But maybe they are needed.

The macro is used as a compile time buffer overflow protection.

>
>> +             struct flow_match_basic match;
>> +
>> +             flow_rule_match_basic(rule, &match);
>> +             // ethertype
>> +             mv88e6xxx_tcam_match_set(key, 16, match.key->n_proto, match.mask->n_proto);
>> +             mv88e6xxx_tcam_match_set(key, 27, match.key->ip_proto, match.mask->ip_proto);
>
>What are these magic numbers?

The offset in the frame used for matching. The Ethertype is at offset 16, the ip protocol at offset 27.

I'm processing your other comments.

Cedric