On 19.08.23 15:29, Vincent Mailhol wrote:
On Sat. 19 Aug. 2023 at 22:10, Vincent Mailhol
<vincent.mailhol@xxxxxxxxx> wrote:
On Sat. 19 Aug. 2023, 01:19, Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
On Thu, 17 Aug 2023 12:10:13 +0200 Martin Hundebøll wrote:
+ int len = nla_len(data[IFLA_CAN_HW_FILTER]);
+ int num_filter = len / sizeof(struct can_filter);
+ struct can_filter *filter = nla_data(data[IFLA_CAN_HW_FILTER]);
This will prevent you from ever extending struct can_filter in
a backward-compatible fashion, right? I obviously know very little
about CAN but are you confident a more bespoke API to manipulate
filters individually and allow extensibility is not warranted?
I follow Jakub's point of view.
The current struct can_filter is not sound. Some devices such as the
ES582.1 supports filtering of the CAN frame based on the flags (i.e.
SFF/EFF, RTR, FDF).
I wrote too fast. The EFF and RTR flags are contained in the canid_t,
so the current struct can_filter is able to handle these two flags.
But it remains true that the CAN-FD flags (FDF and BRS) are currently
not handled. Not to mention that more flags will come with the
upcoming CAN XL.
You are right with FDF where we could use the former CAN_ERR_FLAG value
which is not needed for hw filter API.
But regarding CAN XL we could use the Standard 11 bit ID handling with
another flag inside the remaining 18 bits.
The general concept of re-using the struct can_filter makes sense to me
as this follows the widely used pattern in the af_can.c core and CAN_RAW
sockets.
Best regards,
Oliver
I think that each of the fields of the filter should have its own NLA
declaration with the whole thing wrapped within a NLA_NESTED_ARRAY.
I also think that there should then be a method to report the precise
filtering capabilities of the hardware.
Yours sincerely,
Vincent Mailhol
