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

[Andrew Lunn]

On Tue, Feb 17, 2026 at 03:55:02PM +0000, Cedric Jehasse wrote:
> 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.

Functions are better, so that you get the compiler doing type
checking. But the typeof(_offset) suggests you might be passing
different types? But offset should just be an unsigned int? As i said,
i want to look in more detail.

And i don't see why __mv88e6xxx_tcam_match_set() cannot be a normal
function, and let the compile decide if it should be inlined. There is
nothing performance sensitive here, this is not the hot path.

> >> +             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.

These are the sort of numbers which probably appear other places, and
so might have #defines for them. Otherwise, please do add local
#defines.

	Andrew