All of lore.kernel.org
 help / color / mirror / Atom feed
* Framework for hardware filter support?
@ 2018-02-11  2:38 Daniel Santos
  2018-02-11 12:32 ` Oliver Hartkopp
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Santos @ 2018-02-11  2:38 UTC (permalink / raw)
  To: linux-can

Hello.  I'm doing a project where hardware filters will pretty much be
required unless we change the hardware.  I can just hack up the mcp251x
driver and add some sysfs nodes to modify the filters, but it would be
much nicer to implement using some type of standard framework that I
could submit back upstream.

My current project uses an MCP2515 (with only two rx buffers) connected
via an MCP2210 USB-to-SPI bridge and that introduces a lot of latency
and interrupts may not be serviced for as much as 20-30 ms and w/o
hardware filters, we'll be missing a lot of messages we want to sniff.

I've read some of the arguments against it, such is it being a
multi-user environment.  To me that just means that hardware filters
should be configured at the interface level and not at the socket level.

So is there no such framework already in place?  If not, where would be
the most logical place to add it? netfilter in the kernel and iproute2
in userland?

Thanks,
Daniel



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Framework for hardware filter support?
  2018-02-11  2:38 Framework for hardware filter support? Daniel Santos
@ 2018-02-11 12:32 ` Oliver Hartkopp
  2018-02-11 22:33   ` Daniel Santos
  0 siblings, 1 reply; 7+ messages in thread
From: Oliver Hartkopp @ 2018-02-11 12:32 UTC (permalink / raw)
  To: Daniel Santos; +Cc: linux-can

Hello Daniel,

On 02/11/2018 03:38 AM, Daniel Santos wrote:
> Hello.  I'm doing a project where hardware filters will pretty much be
> required unless we change the hardware.  I can just hack up the mcp251x
> driver and add some sysfs nodes to modify the filters, but it would be
> much nicer to implement using some type of standard framework that I
> could submit back upstream.

Definitely a better approach :-)

> My current project uses an MCP2515 (with only two rx buffers) connected
> via an MCP2210 USB-to-SPI bridge and that introduces a lot of latency
> and interrupts may not be serviced for as much as 20-30 ms and w/o
> hardware filters, we'll be missing a lot of messages we want to sniff.
> 
> I've read some of the arguments against it, such is it being a
> multi-user environment.  To me that just means that hardware filters
> should be configured at the interface level and not at the socket level.

Correct for your use-case.

As you already pointed out in the user in a multi-user environment 
expects to be able to access the entire CAN traffic on that bus.

You want to reduce bandwidth and system load on the path from the 
controller into the system to cope with hardware limitations.

IMO this can be a valid requirement BUT we always have to keep in mind 
that the filters on that (CAN controller) level affect all users.

> So is there no such framework already in place?  If not, where would be
> the most logical place to add it? netfilter in the kernel and iproute2
> in userland?

A hardware filter support needs to be placed in hardware ;-)

The netfilter stuff comes into action when the network packets are 
already processed inside the network layer - so it's too late.

I would suggest to create an add-on to the existing netlink 
configuration interface for CAN controllers (the iproute2 stuff).

The question is, how to build a common configuration interface for CAN 
hardware filters that fits the design of the CAN controllers hardware 
filters?!?

Would it make sense to implement a similar interface like CAN_RAW filters

http://elixir.free-electrons.com/linux/v4.15.2/source/Documentation/networking/can.txt#L446

for the configuration of hardware filters?

E.g. the SJA1000 controller only has exactly ONE of these filters

http://elixir.free-electrons.com/linux/v4.15.2/source/drivers/net/can/sja1000/sja1000.c#L190

where the CAN_RAW filter design would easily fit.

E.g. the MCP2515 filters look a bit different on page 33:
http://ww1.microchip.com/downloads/en/DeviceDoc/21801e.pdf

but the CAN_RAW filter set idea seems to be applicable too.

Would this approach match your requirements?

Best regards,
Oliver

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Framework for hardware filter support?
  2018-02-11 12:32 ` Oliver Hartkopp
@ 2018-02-11 22:33   ` Daniel Santos
  2018-02-13 19:42     ` Kurt Van Dijck
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Santos @ 2018-02-11 22:33 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: linux-can

Thanks for your response Oliver!


On 02/11/2018 06:32 AM, Oliver Hartkopp wrote:
> Hello Daniel,
>
> On 02/11/2018 03:38 AM, Daniel Santos wrote:
>> Hello.  I'm doing a project where hardware filters will pretty much be
>> required unless we change the hardware.  I can just hack up the mcp251x
>> driver and add some sysfs nodes to modify the filters, but it would be
>> much nicer to implement using some type of standard framework that I
>> could submit back upstream.
>
> Definitely a better approach :-)

Well I'm on the clock for this functionality, so I've added a sysfs
interface to do this.  If it's too ugly for upstream, then I'll have to
have my client post the sources somewhere. :-)  I'm not sure they'll be
interested in paying me to make this pretty, lol!

>> My current project uses an MCP2515 (with only two rx buffers) connected
>> via an MCP2210 USB-to-SPI bridge and that introduces a lot of latency
>> and interrupts may not be serviced for as much as 20-30 ms and w/o
>> hardware filters, we'll be missing a lot of messages we want to sniff.
>>
>> I've read some of the arguments against it, such is it being a
>> multi-user environment.  To me that just means that hardware filters
>> should be configured at the interface level and not at the socket level.
>
> Correct for your use-case.
>
> As you already pointed out in the user in a multi-user environment
> expects to be able to access the entire CAN traffic on that bus.
>
> You want to reduce bandwidth and system load on the path from the
> controller into the system to cope with hardware limitations.
>
> IMO this can be a valid requirement BUT we always have to keep in mind
> that the filters on that (CAN controller) level affect all users.

So I haven't dug into SocketCAN yet -- that part of my project is next,
but my first thought was that this layer could allow the user to know
what hardware filters are in place (that they cannot change).  So that
the interface would have both its current configurable software filters
in addition to read-only hardware filters.  That could feasibly save
programmers some headaches down the road.

>> So is there no such framework already in place?  If not, where would be
>> the most logical place to add it? netfilter in the kernel and iproute2
>> in userland?
>
> A hardware filter support needs to be placed in hardware ;-)
>
> The netfilter stuff comes into action when the network packets are
> already processed inside the network layer - so it's too late.

Oh yeah, I forgot about that! :) I used a lot of netfilter long ago when
I had to host a server for something.

> I would suggest to create an add-on to the existing netlink
> configuration interface for CAN controllers (the iproute2 stuff).
>
> The question is, how to build a common configuration interface for CAN
> hardware filters that fits the design of the CAN controllers hardware
> filters?!?

Yes, I've thought about this.  I will have to study the hardware
filtering mechanisms of a number of CAN controllers to design anything sane.

> Would it make sense to implement a similar interface like CAN_RAW filters
>
> http://elixir.free-electrons.com/linux/v4.15.2/source/Documentation/networking/can.txt#L446
>
>
> for the configuration of hardware filters?
>
> E.g. the SJA1000 controller only has exactly ONE of these filters
>
> http://elixir.free-electrons.com/linux/v4.15.2/source/drivers/net/can/sja1000/sja1000.c#L190
>
>
> where the CAN_RAW filter design would easily fit.

This is what I mean by a framework.  So the driver should inform the
core CAN driver what it's filtering capabilities are -- again, I would
have to study a number of devices to design this sanely.  It would seem
that iproute2 would be the best userland mechanism to specify them, and
it could pass an array of struct can_filter objects.  Then based upon
the hardware, it can apply or reject them.  It would also be nice for
iproute2 to query the driver and be able to report those capabilities.

> E.g. the MCP2515 filters look a bit different on page 33:
> http://ww1.microchip.com/downloads/en/DeviceDoc/21801e.pdf

Yes, this is a good example of an oddity.  It has two rx buffers, the
first of higher priority.  RXB0 can optionally use a single mask and two
filters, RXB1 can use a single mask and 4 filters.  RXB0 can also be
configured so that if it matches a message when it already has one (that
the MCU hasn't retrieved yet), it rolls it over to RXB1, even
overwriting any message that was matched from RXB1's own filters.

That's not very straight-forward, but aside from clarifying rather or
not to rollover, an array of 1-6 struct can_filter objects can be used. 
If the first two masks are the same, it can use the common mask and two
filters for RXB0, then if the remaining 4 masks are the same, then it
can similarly init the RXB1 filters.  What I need to learn is just how
diverse this is in the hardware that's out there.

In the mcp251x driver's current state, it always enables this rollover
because without using filters, it means it can rx two messages before
overflowing.

> but the CAN_RAW filter set idea seems to be applicable too.
>
> Would this approach match your requirements?

I think we're getting close.  I certainly understand the situation much
better now.  I suppose I'll just have to come up with something and
propose it.

> Best regards,
> Oliver
>

Thanks!
Daniel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Framework for hardware filter support?
  2018-02-11 22:33   ` Daniel Santos
@ 2018-02-13 19:42     ` Kurt Van Dijck
  2018-02-13 20:44       ` Oliver Hartkopp
  2018-02-14 22:40       ` Daniel Santos
  0 siblings, 2 replies; 7+ messages in thread
From: Kurt Van Dijck @ 2018-02-13 19:42 UTC (permalink / raw)
  To: Daniel Santos; +Cc: Oliver Hartkopp, linux-can

On zo, 11 feb 2018 16:33:43 -0600, Daniel Santos wrote:
> > where the CAN_RAW filter design would easily fit.
> 
> This is what I mean by a framework.  So the driver should inform the
> core CAN driver what it's filtering capabilities are -- again, I would
> have to study a number of devices to design this sanely.  It would seem
> that iproute2 would be the best userland mechanism to specify them, and
> it could pass an array of struct can_filter objects.  Then based upon
> the hardware, it can apply or reject them.  It would also be nice for
> iproute2 to query the driver and be able to report those capabilities.

I remember this discussion has passed a few times already.
The end result has always been to:
1. give the kernel/driver a list of id/masks.
2. The driver does no aggregation, the root operator should know
   what to do.

Maybe some day this will become limited for hardware that can do special
tricks, like having a random list of identifiers.
This can most probable be addressed in some clever way that is specific
for the driver (like adding a list of id/masks where mask is always
0x7ff for a random canid list).
Due to the driver doing smart things, the driver may not be capable of
informing filtering capabilities in a static way. I would start without
it.

The support of hardware filters is hardware specific anyway, and
intersects the multi-user principle. The only justification is ....
that the root operator knows the hardware and the application.

What matters for a generic implementation is that the API is simple,
clear and usable. That is where the id/mask list pops in.
Operators that don't understand the id/mask list will probably fail
anyway to use hardware filters on CAN. No framework can prevent that.

Kurt

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Framework for hardware filter support?
  2018-02-13 19:42     ` Kurt Van Dijck
@ 2018-02-13 20:44       ` Oliver Hartkopp
  2018-02-14 22:40       ` Daniel Santos
  1 sibling, 0 replies; 7+ messages in thread
From: Oliver Hartkopp @ 2018-02-13 20:44 UTC (permalink / raw)
  To: Daniel Santos, Kurt Van Dijck; +Cc: linux-can

Hi Kurt,

On 02/13/2018 08:42 PM, Kurt Van Dijck wrote:
> On zo, 11 feb 2018 16:33:43 -0600, Daniel Santos wrote:
>>> where the CAN_RAW filter design would easily fit.
>>
>> This is what I mean by a framework.  So the driver should inform the
>> core CAN driver what it's filtering capabilities are -- again, I would
>> have to study a number of devices to design this sanely.  It would seem
>> that iproute2 would be the best userland mechanism to specify them, and
>> it could pass an array of struct can_filter objects.  Then based upon
>> the hardware, it can apply or reject them.  It would also be nice for
>> iproute2 to query the driver and be able to report those capabilities.
> 
> I remember this discussion has passed a few times already.
> The end result has always been to:
> 1. give the kernel/driver a list of id/masks.
> 2. The driver does no aggregation, the root operator should know
>     what to do.
> 
> Maybe some day this will become limited for hardware that can do special
> tricks, like having a random list of identifiers.
> This can most probable be addressed in some clever way that is specific
> for the driver (like adding a list of id/masks where mask is always
> 0x7ff for a random canid list).
> Due to the driver doing smart things, the driver may not be capable of
> informing filtering capabilities in a static way. I would start without
> it.

Ack.

> The support of hardware filters is hardware specific anyway, and
> intersects the multi-user principle. The only justification is ....
> that the root operator knows the hardware and the application.
> 
> What matters for a generic implementation is that the API is simple,
> clear and usable. That is where the id/mask list pops in.
> Operators that don't understand the id/mask list will probably fail
> anyway to use hardware filters on CAN. No framework can prevent that.
> 

Full ACK!

I'm currently looking into netlink how to pass a larger number of struct 
can_filter objects to the CAN driver.

I've checked the M_CAN IP core and the RX filters can be up to 128 
standard ID filters and up to 64 extended ID filters where latter seem 
to share a common mask.

There is some NLA_NESTED data type which seems to be the right way to 
transfer arrays of can_filter's - but I did not get completely behind it 
yet ;-)

Best,
Oliver

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Framework for hardware filter support?
  2018-02-13 19:42     ` Kurt Van Dijck
  2018-02-13 20:44       ` Oliver Hartkopp
@ 2018-02-14 22:40       ` Daniel Santos
  2018-02-15 23:03         ` Kurt Van Dijck
  1 sibling, 1 reply; 7+ messages in thread
From: Daniel Santos @ 2018-02-14 22:40 UTC (permalink / raw)
  To: Kurt Van Dijck; +Cc: Oliver Hartkopp, linux-can

Thanks for your response, Kurt.


On 02/13/2018 01:42 PM, Kurt Van Dijck wrote:
> On zo, 11 feb 2018 16:33:43 -0600, Daniel Santos wrote:
>>> where the CAN_RAW filter design would easily fit.
>> This is what I mean by a framework.  So the driver should inform the
>> core CAN driver what it's filtering capabilities are -- again, I would
>> have to study a number of devices to design this sanely.  It would seem
>> that iproute2 would be the best userland mechanism to specify them, and
>> it could pass an array of struct can_filter objects.  Then based upon
>> the hardware, it can apply or reject them.  It would also be nice for
>> iproute2 to query the driver and be able to report those capabilities.
> I remember this discussion has passed a few times already.
> The end result has always been to:
> 1. give the kernel/driver a list of id/masks.
> 2. The driver does no aggregation, the root operator should know
>    what to do.

I don't understand what driver aggregation means.  But yes, this makes
good sense.

> Maybe some day this will become limited for hardware that can do special
> tricks, like having a random list of identifiers.
> This can most probable be addressed in some clever way that is specific
> for the driver (like adding a list of id/masks where mask is always
> 0x7ff for a random canid list).
> Due to the driver doing smart things, the driver may not be capable of
> informing filtering capabilities in a static way. I would start without
> it.
>
> The support of hardware filters is hardware specific anyway, and
> intersects the multi-user principle. The only justification is ....
> that the root operator knows the hardware and the application.

Yes, perfect!

> What matters for a generic implementation is that the API is simple,
> clear and usable. That is where the id/mask list pops in.
> Operators that don't understand the id/mask list will probably fail
> anyway to use hardware filters on CAN. No framework can prevent that.
>
> Kurt
>

Yes, this makes great sense. And the driver (I presume) can always
reject the filter configuration if it can't do what is asked.  In the
case of the MCP2210, however, there is one small hole; it has a bit that
decides rather or not to rollover messages from it's first rx buffer to
it's second (and it only has two).  I'm wondering about some free-form,
driver specific configuration string to handle any loose ends of the
device?  Too ugly?

Then again, it supports 6 filters.  Perhaps a 7th filter could be passed
to specify the rollover configuration.  Uglier still? :)

And actually, the device has bits to en/disable filtering for each rx
buffer that can be changed (along with the rollover) while the link is
up.  I'm probably *really* reaching here.  I can't think of many reasons
to have to change this while the link is up and rather or not they are
enabled can be implied by the filter/mask array.

Thanks,
Daniel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Framework for hardware filter support?
  2018-02-14 22:40       ` Daniel Santos
@ 2018-02-15 23:03         ` Kurt Van Dijck
  0 siblings, 0 replies; 7+ messages in thread
From: Kurt Van Dijck @ 2018-02-15 23:03 UTC (permalink / raw)
  To: Daniel Santos; +Cc: Oliver Hartkopp, linux-can



On 14 February 2018 23:40:19 CET, Daniel Santos <daniel.santos@pobox.com> wrote:
>Thanks for your response, Kurt.
>
>
>On 02/13/2018 01:42 PM, Kurt Van Dijck wrote:
>> On zo, 11 feb 2018 16:33:43 -0600, Daniel Santos wrote:
>>>> where the CAN_RAW filter design would easily fit.
>>> This is what I mean by a framework.  So the driver should inform the
>>> core CAN driver what it's filtering capabilities are -- again, I
>would
>>> have to study a number of devices to design this sanely.  It would
>seem
>>> that iproute2 would be the best userland mechanism to specify them,
>and
>>> it could pass an array of struct can_filter objects.  Then based
>upon
>>> the hardware, it can apply or reject them.  It would also be nice
>for
>>> iproute2 to query the driver and be able to report those
>capabilities.
>> I remember this discussion has passed a few times already.
>> The end result has always been to:
>> 1. give the kernel/driver a list of id/masks.
>> 2. The driver does no aggregation, the root operator should know
>>    what to do.
>
>I don't understand what driver aggregation means.  But yes, this makes
>good sense.

I meant with aggregation: any form of accumulation across different calls

>> Maybe some day this will become limited for hardware that can do
>special
>> tricks, like having a random list of identifiers.
>> This can most probable be addressed in some clever way that is
>specific
>> for the driver (like adding a list of id/masks where mask is always
>> 0x7ff for a random canid list).
>> Due to the driver doing smart things, the driver may not be capable
>of
>> informing filtering capabilities in a static way. I would start
>without
>> it.
>>
>> The support of hardware filters is hardware specific anyway, and
>> intersects the multi-user principle. The only justification is ....
>> that the root operator knows the hardware and the application.
>
>Yes, perfect!
>
>> What matters for a generic implementation is that the API is simple,
>> clear and usable. That is where the id/mask list pops in.
>> Operators that don't understand the id/mask list will probably fail
>> anyway to use hardware filters on CAN. No framework can prevent that.
>>
>> Kurt
>>
>
>Yes, this makes great sense. And the driver (I presume) can always
>reject the filter configuration if it can't do what is asked.  In the

Yep, that's it

>case of the MCP2210, however, there is one small hole; it has a bit
>that
>decides rather or not to rollover messages from it's first rx buffer to
>it's second (and it only has two).  I'm wondering about some free-form,
>driver specific configuration string to handle any loose ends of the
>device?  Too ugly?
>
>Then again, it supports 6 filters.  Perhaps a 7th filter could be
>passed
>to specify the rollover configuration.  Uglier still? :)
>
>And actually, the device has bits to en/disable filtering for each rx
>buffer that can be changed (along with the rollover) while the link is
>up.  I'm probably *really* reaching here.  I can't think of many
>reasons
>to have to change this while the link is up and rather or not they are
>enabled can be implied by the filter/mask array.

A driver, and for sure an API, does not necessarily export all dirty bits to userspace. There will be things that are possible in hardware that simply cannot be exported in a comprehensible way.
The need for hardware filters does not imply a need to modify them in runtime.
Roll-over and other silicon magic is IMHO best preset to a default that makes most sense, or addressed in sysfs attributes, if you think that it must be accessible.

>
>Thanks,
>Daniel
Kurt

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-02-15 23:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-11  2:38 Framework for hardware filter support? Daniel Santos
2018-02-11 12:32 ` Oliver Hartkopp
2018-02-11 22:33   ` Daniel Santos
2018-02-13 19:42     ` Kurt Van Dijck
2018-02-13 20:44       ` Oliver Hartkopp
2018-02-14 22:40       ` Daniel Santos
2018-02-15 23:03         ` Kurt Van Dijck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.