[PATCH v2 0/3] net: Linn Ethernet Packet Sniffer driver

[PATCH v2 0/3] net: Linn Ethernet Packet Sniffer driver

[Stathis Voukelatos]

This patch adds support the Ethernet Packet Sniffer H/W module
developed by Linn Products Ltd and found in the IMG Pistachio SoC.
The module allows Ethernet packets to be parsed, matched against
a user-defined pattern and timestamped. It sits between a 100M
Ethernet MAC and PHY and is completely passive with respect to
Ethernet frames.

Matched packet bytes and timestamp values are returned through a
FIFO. Timestamps are provided to the module through an externally
generated Gray-encoded counter.

The command string for packet matching is stored in module RAM
and consists of a sequence of 16-bit entries. Each entry includes
an 8-bit command code and and 8-bit data value. Valid command
codes are:
0 - Don't care
1 - Match: packet data must match command string byte
2 - Copy: packet data will be copied to FIFO
3 - Match/Stamp: if packet data matches string byte, a timestamp
                 is copied into the FIFO
4 - Copy/Done: packet data will be copied into the FIFO.
               This command terminates the command string.

The driver consists of two modules:
- Core: it provides a common framework for managing backend packet
        sniffer implementations. Each backend channel is registered
        by the core as a netdev, which can be accessed from user
        space through a raw packet socket. 

- Ethernet Packet Sniffer backend: provides the driver for the
        Linn Ethernet Packet Sniffer H/W modules.

The split between a core and backend modules allows for other
implementations to be added in the future apart of the Ethernet
packet sniffer presented in this patch set.

Changes:
v2:
* Complete redesign of core framework to use netdev instead of 
the generic netlink framework
* Updated device tree binding
* A number of minor code improvements suggested by code review

Stathis Voukelatos (3):
  Ethernet packet sniffer: Device tree binding and vendor prefix
  Packet sniffer core framework
  Linn Ethernet packet sniffer driver

 .../bindings/net/linn-ether-packet-sniffer.txt     |  42 +++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 MAINTAINERS                                        |   6 +
 drivers/net/Kconfig                                |   2 +
 drivers/net/Makefile                               |   2 +
 drivers/net/pkt-sniffer/Kconfig                    |  19 +
 drivers/net/pkt-sniffer/Makefile                   |   7 +
 drivers/net/pkt-sniffer/backends/ether/channel.c   | 392 +++++++++++++++++++++
 drivers/net/pkt-sniffer/backends/ether/channel.h   |  81 +++++
 drivers/net/pkt-sniffer/backends/ether/hw.h        |  46 +++
 drivers/net/pkt-sniffer/backends/ether/platform.c  | 301 ++++++++++++++++
 drivers/net/pkt-sniffer/core/module.c              |  37 ++
 drivers/net/pkt-sniffer/core/netdev.c              | 254 +++++++++++++
 drivers/net/pkt-sniffer/core/snf_core.h            |  60 ++++
 include/uapi/linux/pkt_sniffer.h                   |  33 ++
 15 files changed, 1283 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
 create mode 100644 drivers/net/pkt-sniffer/Kconfig
 create mode 100644 drivers/net/pkt-sniffer/Makefile
 create mode 100644 drivers/net/pkt-sniffer/backends/ether/channel.c
 create mode 100644 drivers/net/pkt-sniffer/backends/ether/channel.h
 create mode 100644 drivers/net/pkt-sniffer/backends/ether/hw.h
 create mode 100644 drivers/net/pkt-sniffer/backends/ether/platform.c
 create mode 100644 drivers/net/pkt-sniffer/core/module.c
 create mode 100644 drivers/net/pkt-sniffer/core/netdev.c
 create mode 100644 drivers/net/pkt-sniffer/core/snf_core.h
 create mode 100644 include/uapi/linux/pkt_sniffer.h

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix

[Stathis Voukelatos]

Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
---
 .../bindings/net/linn-ether-packet-sniffer.txt     | 42 ++++++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt        |  1 +
 2 files changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt

diff --git a/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt b/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
new file mode 100644
index 0000000..74bac5e
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
@@ -0,0 +1,42 @@
+* Linn Products Ethernet Packet Sniffer
+The module allows Ethernet packets to be parsed, matched against
+a user-defined pattern and timestamped. It sits between a 100M
+Ethernet MAC and PHY and is completely passive with respect to
+Ethernet frames.
+Matched packet bytes and timestamp values are returned through a
+FIFO. Timestamps are provided to the module through an externally
+generated Gray-encoded counter.
+
+Required properties:
+- compatible : must be "linn,eth-sniffer"
+- reg : physical addresses and sizes of registers. Must contain 3 entries:
+          - registers memory space
+          - TX command string memory
+          - RX command string memory
+- reg-names : must contain the following 3 entries:
+                  "regs", "tx-ram", "rx-ram"
+- interrupts : sniffer interrupt specifier
+- clocks : specify the system clock for the peripheral and
+	   the enable clock for the timestamp counter
+- clock-names : must contain the "sys" and "tstamp" entries
+- fifo-block-words : number of words in one data FIFO entry
+- tstamp-hz : frequency of the timestamp counter
+- tstamp-shift : shift value for the timestamp cyclecounter struct
+- tstamp-bits : width in bits of the timestamp counter
+
+Example:
+
+sniffer@1814a000 {
+	compatible = "linn,eth-sniffer";
+	reg = <0x1814a000 0x100>, <0x1814a400 0x400>,
+	      <0x1814a800 0x400>;
+	reg-names = "regs", "tx-ram", "rx-ram";
+	interrupts = <GIC_SHARED 58 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&clk_core CLK_AUDIO>,
+		 <&cr_periph SYS_CLK_EVENT_TIMER>;
+	clock-names = "sys", "tstamp";
+	fifo-block-words = <4>;
+	tstamp-hz = <52000000>;
+	tstamp-shift = <27>;
+	tstamp-bits = <30>;
+};
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index d443279..891c224 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -90,6 +90,7 @@ lacie	LaCie
 lantiq	Lantiq Semiconductor
 lenovo	Lenovo Group Ltd.
 lg	LG Corporation
+linn    Linn Products Ltd.
 linux	Linux-specific binding
 lsi	LSI Corp. (LSI Logic)
 lltc	Linear Technology Corporation
-- 
1.9.1

Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix

[Sergei Shtylyov]

Hello.

On 02/17/2015 05:03 PM, Stathis Voukelatos wrote:

> Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>

[...]

> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index d443279..891c224 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -90,6 +90,7 @@ lacie	LaCie
>   lantiq	Lantiq Semiconductor
>   lenovo	Lenovo Group Ltd.
>   lg	LG Corporation
> +linn    Linn Products Ltd.

    Please use tab for indenting the company name.

>   linux	Linux-specific binding
>   lsi	LSI Corp. (LSI Logic)
>   lltc	Linear Technology Corporation

WBR, Sergei

Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix

[Mark Rutland]

On Tue, Feb 17, 2015 at 02:03:31PM +0000, Stathis Voukelatos wrote:
> Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
> ---
>  .../bindings/net/linn-ether-packet-sniffer.txt     | 42 ++++++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>  2 files changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt b/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
> new file mode 100644
> index 0000000..74bac5e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
> @@ -0,0 +1,42 @@
> +* Linn Products Ethernet Packet Sniffer
> +The module allows Ethernet packets to be parsed, matched against
> +a user-defined pattern and timestamped. It sits between a 100M
> +Ethernet MAC and PHY and is completely passive with respect to
> +Ethernet frames.
> +Matched packet bytes and timestamp values are returned through a
> +FIFO. Timestamps are provided to the module through an externally
> +generated Gray-encoded counter.

Does this counter unit need to be enabled (or have any input clocks
enabled)?

> +
> +Required properties:
> +- compatible : must be "linn,eth-sniffer"

Is there not a more precise name for this IP block?

> +- reg : physical addresses and sizes of registers. Must contain 3 entries:
> +          - registers memory space
> +          - TX command string memory
> +          - RX command string memory
> +- reg-names : must contain the following 3 entries:
> +                  "regs", "tx-ram", "rx-ram"

It would be nicer to format this as:

- reg: A list of physical address and size pairs corresponding to each
       entry in reg-names

- reg-names: must contain:
  * "regs" for the control registers
  * "tx-ram" for the TX command string memory
  * "rx-ram" for the RX command string memory

Which avoids redundancy.

The phrase "command string" sounds a bit odd. What are these used for
exactly?

> +- interrupts : sniffer interrupt specifier
> +- clocks : specify the system clock for the peripheral and
> +	   the enable clock for the timestamp counter
> +- clock-names : must contain the "sys" and "tstamp" entries

Similarly here clocks should just be defined in terms of clock-names.

> +- fifo-block-words : number of words in one data FIFO entry

I didn't see a data FIFO described. Is that dynamically allocated and
handed to the sniffer, or does that correspond to one of the memory
regions above?

> +- tstamp-hz : frequency of the timestamp counter
> +- tstamp-shift : shift value for the timestamp cyclecounter struct

What exactly is this used for?

Are there any docs?

> +- tstamp-bits : width in bits of the timestamp counter
> +
> +Example:
> +
> +sniffer@1814a000 {
> +	compatible = "linn,eth-sniffer";
> +	reg = <0x1814a000 0x100>, <0x1814a400 0x400>,
> +	      <0x1814a800 0x400>;
> +	reg-names = "regs", "tx-ram", "rx-ram";
> +	interrupts = <GIC_SHARED 58 IRQ_TYPE_LEVEL_HIGH>;
> +	clocks = <&clk_core CLK_AUDIO>,
> +		 <&cr_periph SYS_CLK_EVENT_TIMER>;
> +	clock-names = "sys", "tstamp";
> +	fifo-block-words = <4>;
> +	tstamp-hz = <52000000>;
> +	tstamp-shift = <27>;
> +	tstamp-bits = <30>;

This property wasn't documented.

As mentioned previously, I think the relation between this unit and the
MAC and/or PHY needs to be explicitly described in the DT.

> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index d443279..891c224 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -90,6 +90,7 @@ lacie	LaCie
>  lantiq	Lantiq Semiconductor
>  lenovo	Lenovo Group Ltd.
>  lg	LG Corporation
> +linn    Linn Products Ltd.

This addition looks fine to me. For some reason it seems to be padded
with spaces instead of tabs though; is my mail server corrupting things
or is that the case in the original patch?

Thanks,
Mark.

Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix

[Stathis Voukelatos]

Hi Mark,

On 17/02/15 14:51, Mark Rutland wrote:

>> +Matched packet bytes and timestamp values are returned through a
>> +FIFO. Timestamps are provided to the module through an externally
>> +generated Gray-encoded counter.
>
> Does this counter unit need to be enabled (or have any input clocks
> enabled)?
>

Yes it does, that is the purpose of the 'tstamp' entry in the
'clock-names' property below.

>> +
>> +Required properties:
>> +- compatible : must be "linn,eth-sniffer"
>
> Is there not a more precise name for this IP block?

It is generally called 'ethernet packet sniffer', maybe
linn,eth-packet-sniffer would be a more descriptive name?

>
>> +- reg : physical addresses and sizes of registers. Must contain 3 entries:
>> +          - registers memory space
>> +          - TX command string memory
>> +          - RX command string memory
>> +- reg-names : must contain the following 3 entries:
>> +                  "regs", "tx-ram", "rx-ram"
>
> It would be nicer to format this as:
>
> - reg: A list of physical address and size pairs corresponding to each
>         entry in reg-names
>
> - reg-names: must contain:
>    * "regs" for the control registers
>    * "tx-ram" for the TX command string memory
>    * "rx-ram" for the RX command string memory
>
> Which avoids redundancy.
>

Will change formatting as suggested

> The phrase "command string" sounds a bit odd. What are these used for
> exactly?

In these two memory areas we program a sequence of bytes in the
format: [cmd][value][cmd][value] to configure the data patterns that
the sniffer should match for Ethernet TX and RX packets respectively.
Maybe 'command memory' would be clearer?

>
>> +- interrupts : sniffer interrupt specifier
>> +- clocks : specify the system clock for the peripheral and
>> +	   the enable clock for the timestamp counter
>> +- clock-names : must contain the "sys" and "tstamp" entries
>
> Similarly here clocks should just be defined in terms of clock-names.

Will reformat similar to the 'regs' field

>
>> +- fifo-block-words : number of words in one data FIFO entry
>
> I didn't see a data FIFO described. Is that dynamically allocated and
> handed to the sniffer, or does that correspond to one of the memory
> regions above?
>

It is a H/W FIFO internal to the module and accessed through
a register. It is divided in blocks and 'fifo-block-words'
specify the number of words in each block. It is needed by
the driver to make sure it reads an entire block, in order
to clear the 'data available' interrupt.

>> +- tstamp-hz : frequency of the timestamp counter
>> +- tstamp-shift : shift value for the timestamp cyclecounter struct
>
> What exactly is this used for?
>
> Are there any docs?

See: include/linux/clocksource.h
The driver uses a cyclecounter and timecounter to convert raw timestamps
to nanoseconds. 'tstamp-shift' refers to the 'shift' field of the
cyclecounter structure, that can be used to improve the precision of
the conversion

>
>> +- tstamp-bits : width in bits of the timestamp counter
>> +
>> +Example:
>> +
>> +sniffer@1814a000 {
>> +	compatible = "linn,eth-sniffer";
>> +	reg = <0x1814a000 0x100>, <0x1814a400 0x400>,
>> +	      <0x1814a800 0x400>;
>> +	reg-names = "regs", "tx-ram", "rx-ram";
>> +	interrupts = <GIC_SHARED 58 IRQ_TYPE_LEVEL_HIGH>;
>> +	clocks = <&clk_core CLK_AUDIO>,
>> +		 <&cr_periph SYS_CLK_EVENT_TIMER>;
>> +	clock-names = "sys", "tstamp";
>> +	fifo-block-words = <4>;
>> +	tstamp-hz = <52000000>;
>> +	tstamp-shift = <27>;
>> +	tstamp-bits = <30>;
>
> This property wasn't documented.
>
> As mentioned previously, I think the relation between this unit and the
> MAC and/or PHY needs to be explicitly described in the DT.

Do you suggest a field along the lines of:
mac = <&eth_controller_0>;
The driver could check that it exists and is valid but does
not need to make use of it.

>
>> +};
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> index d443279..891c224 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> @@ -90,6 +90,7 @@ lacie	LaCie
>>   lantiq	Lantiq Semiconductor
>>   lenovo	Lenovo Group Ltd.
>>   lg	LG Corporation
>> +linn    Linn Products Ltd.
>
> This addition looks fine to me. For some reason it seems to be padded
> with spaces instead of tabs though; is my mail server corrupting things
> or is that the case in the original patch?

Sorry, it was spaces. Will be fixed
>
> Thanks,
> Mark.
>

Thank you,
Stathis

Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix

[Mark Rutland]

On Tue, Feb 17, 2015 at 04:22:04PM +0000, Stathis Voukelatos wrote:
> Hi Mark,
> 
> On 17/02/15 14:51, Mark Rutland wrote:
> 
> >> +Matched packet bytes and timestamp values are returned through a
> >> +FIFO. Timestamps are provided to the module through an externally
> >> +generated Gray-encoded counter.
> >
> > Does this counter unit need to be enabled (or have any input clocks
> > enabled)?
> >
> 
> Yes it does, that is the purpose of the 'tstamp' entry in the
> 'clock-names' property below.

Ah, I see.

> >> +
> >> +Required properties:
> >> +- compatible : must be "linn,eth-sniffer"
> >
> > Is there not a more precise name for this IP block?
> 
> It is generally called 'ethernet packet sniffer', maybe
> linn,eth-packet-sniffer would be a more descriptive name?

Either way is fine. I was expecting something more like a product code.

[...]

> >> +- fifo-block-words : number of words in one data FIFO entry
> >
> > I didn't see a data FIFO described. Is that dynamically allocated and
> > handed to the sniffer, or does that correspond to one of the memory
> > regions above?
> >
> 
> It is a H/W FIFO internal to the module and accessed through
> a register. It is divided in blocks and 'fifo-block-words'
> specify the number of words in each block. It is needed by
> the driver to make sure it reads an entire block, in order
> to clear the 'data available' interrupt.

I see. I assumed that the FIFO was memory mapped rather than exposed
through a register.

Given the above this sounds fine.

> >> +- tstamp-hz : frequency of the timestamp counter

Is this the frequency the clock is running at, or a frequency that it
should be programmed to in order to be used?

The former can be queried from the common clock framework, and if you
intended the latter the wording shuold be a little more explicit about
that being the case.

> >> +- tstamp-shift : shift value for the timestamp cyclecounter struct
> >
> > What exactly is this used for?
> >
> > Are there any docs?
> 
> See: include/linux/clocksource.h
> The driver uses a cyclecounter and timecounter to convert raw timestamps
> to nanoseconds. 'tstamp-shift' refers to the 'shift' field of the
> cyclecounter structure, that can be used to improve the precision of
> the conversion

Sure, but the very concept of a cyclecounter is a Linux implementation
detail. If we have the frequency of the timer we should be able to
dynamically generate this, so there's no need for this to be in the DT.

> >> +- tstamp-bits : width in bits of the timestamp counter
> >> +
> >> +Example:
> >> +
> >> +sniffer@1814a000 {
> >> +	compatible = "linn,eth-sniffer";
> >> +	reg = <0x1814a000 0x100>, <0x1814a400 0x400>,
> >> +	      <0x1814a800 0x400>;
> >> +	reg-names = "regs", "tx-ram", "rx-ram";
> >> +	interrupts = <GIC_SHARED 58 IRQ_TYPE_LEVEL_HIGH>;
> >> +	clocks = <&clk_core CLK_AUDIO>,
> >> +		 <&cr_periph SYS_CLK_EVENT_TIMER>;
> >> +	clock-names = "sys", "tstamp";
> >> +	fifo-block-words = <4>;
> >> +	tstamp-hz = <52000000>;
> >> +	tstamp-shift = <27>;
> >> +	tstamp-bits = <30>;
> >
> > This property wasn't documented.
> >
> > As mentioned previously, I think the relation between this unit and the
> > MAC and/or PHY needs to be explicitly described in the DT.
> 
> Do you suggest a field along the lines of:
> mac = <&eth_controller_0>;
> The driver could check that it exists and is valid but does
> not need to make use of it.

I would expect some level of the software stack to make use of it, or
you have no idea which ethernet interface is related to this monitoring
interface. Perhaps current systems only have one interface, but that
shouldn't be relied upon.

Thanks,
Mark.

Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix

[Stathis Voukelatos]

Hi Mark,

On 17/02/15 16:35, Mark Rutland wrote:
>
>>>> +- tstamp-hz : frequency of the timestamp counter
>
> Is this the frequency the clock is running at, or a frequency that it
> should be programmed to in order to be used?
>
> The former can be queried from the common clock framework, and if you
> intended the latter the wording shuold be a little more explicit about
> that being the case.
>

It is the frequency of the timestamp values supplied to the sniffer
module. It is used by the driver to convert to nanoseconds.
I was trying to be somewhat generic here and not assume that it
is necessarily the same as the 'tstamp' clock below, in which case we
could indeed obtain it using the common clock framework.

>> See: include/linux/clocksource.h
>> The driver uses a cyclecounter and timecounter to convert raw timestamps
>> to nanoseconds. 'tstamp-shift' refers to the 'shift' field of the
>> cyclecounter structure, that can be used to improve the precision of
>> the conversion
>
> Sure, but the very concept of a cyclecounter is a Linux implementation
> detail. If we have the frequency of the timer we should be able to
> dynamically generate this, so there's no need for this to be in the DT.
>

Most networking driver use hard-coded values for that, but in my case
I did not want to assume a certain fixed clock frequency. I will remove
it from the DT and generate it dynamically. There is a kernel function
clocks_calc_mult_shift() to do it but unfortunately it is not exported,
so I guess I will need to replicate the code.

>>> As mentioned previously, I think the relation between this unit and the
>>> MAC and/or PHY needs to be explicitly described in the DT.
>>
>> Do you suggest a field along the lines of:
>> mac = <&eth_controller_0>;
>> The driver could check that it exists and is valid but does
>> not need to make use of it.
>
> I would expect some level of the software stack to make use of it, or
> you have no idea which ethernet interface is related to this monitoring
> interface. Perhaps current systems only have one interface, but that
> shouldn't be relied upon.

Yes, but the sniffer module is hard-wired to a certain Ethernet Mii
interface. We can add an entry to tie it to an Ethernet controller, but
apart of a sanity check I am not sure what else the S/W can do.

>
> Thanks,
> Mark.
>

Thank you,
Stathis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix

[Mark Rutland]

On Tue, Feb 17, 2015 at 05:13:19PM +0000, Stathis Voukelatos wrote:
> Hi Mark,
> 
> On 17/02/15 16:35, Mark Rutland wrote:
> >
> >>>> +- tstamp-hz : frequency of the timestamp counter
> >
> > Is this the frequency the clock is running at, or a frequency that it
> > should be programmed to in order to be used?
> >
> > The former can be queried from the common clock framework, and if you
> > intended the latter the wording shuold be a little more explicit about
> > that being the case.
> >
> 
> It is the frequency of the timestamp values supplied to the sniffer
> module. It is used by the driver to convert to nanoseconds.
> I was trying to be somewhat generic here and not assume that it
> is necessarily the same as the 'tstamp' clock below, in which case we
> could indeed obtain it using the common clock framework.

In what cases would it _not_ be the same? From your description this is
that clock, no?

> >> See: include/linux/clocksource.h
> >> The driver uses a cyclecounter and timecounter to convert raw timestamps
> >> to nanoseconds. 'tstamp-shift' refers to the 'shift' field of the
> >> cyclecounter structure, that can be used to improve the precision of
> >> the conversion
> >
> > Sure, but the very concept of a cyclecounter is a Linux implementation
> > detail. If we have the frequency of the timer we should be able to
> > dynamically generate this, so there's no need for this to be in the DT.
> >
> 
> Most networking driver use hard-coded values for that, but in my case
> I did not want to assume a certain fixed clock frequency. I will remove
> it from the DT and generate it dynamically. There is a kernel function
> clocks_calc_mult_shift() to do it but unfortunately it is not exported,
> so I guess I will need to replicate the code.

Or submit a patch exporting it, along with the rationale for doing so?

> >>> As mentioned previously, I think the relation between this unit and the
> >>> MAC and/or PHY needs to be explicitly described in the DT.
> >>
> >> Do you suggest a field along the lines of:
> >> mac = <&eth_controller_0>;
> >> The driver could check that it exists and is valid but does
> >> not need to make use of it.
> >
> > I would expect some level of the software stack to make use of it, or
> > you have no idea which ethernet interface is related to this monitoring
> > interface. Perhaps current systems only have one interface, but that
> > shouldn't be relied upon.
> 
> Yes, but the sniffer module is hard-wired to a certain Ethernet Mii
> interface. We can add an entry to tie it to an Ethernet controller, but
> apart of a sanity check I am not sure what else the S/W can do.

Fundamentally, the use-case for this is monitoring an ethernet
interface. So regardless of which kernel framework this plumbs into,
there needs to be a way to go from ethN to whatever this is exposed as.

Exposing a completely separate interface makes no sense. Singleton stuff
like that inevitably gets broken as someone later builds a board with
multiple instances of some similar IP block.

So I would imagine that either the link between interface and monitoring
interface would be described somewhere in the filesystem, or there'd be
a syscall/ioctl/whatever to go from an interface to the appropriate
monitoring interface.

That all depends on exactly how this gets exposed in the end, however.

Thanks,
Mark.

Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix

[Stathis Voukelatos]

On 17/02/15 17:30, Mark Rutland wrote:

>> It is the frequency of the timestamp values supplied to the sniffer
>> module. It is used by the driver to convert to nanoseconds.
>> I was trying to be somewhat generic here and not assume that it
>> is necessarily the same as the 'tstamp' clock below, in which case we
>> could indeed obtain it using the common clock framework.
>
> In what cases would it _not_ be the same? From your description this is
> that clock, no?
>

Counters can often have a divider applied to their input clock and
therefore run at a scaled down frequency. This is not the case in the
first SoC where the sniffer is used, so for simplicity I can modify as
you suggest and remove that field from the DT.

>> Most networking driver use hard-coded values for that, but in my case
>> I did not want to assume a certain fixed clock frequency. I will remove
>> it from the DT and generate it dynamically. There is a kernel function
>> clocks_calc_mult_shift() to do it but unfortunately it is not exported,
>> so I guess I will need to replicate the code.
>
> Or submit a patch exporting it, along with the rationale for doing so?
>

Will do that.

>> Yes, but the sniffer module is hard-wired to a certain Ethernet Mii
>> interface. We can add an entry to tie it to an Ethernet controller, but
>> apart of a sanity check I am not sure what else the S/W can do.
>
> Fundamentally, the use-case for this is monitoring an ethernet
> interface. So regardless of which kernel framework this plumbs into,
> there needs to be a way to go from ethN to whatever this is exposed as.
>
> Exposing a completely separate interface makes no sense. Singleton stuff
> like that inevitably gets broken as someone later builds a board with
> multiple instances of some similar IP block.
>
> So I would imagine that either the link between interface and monitoring
> interface would be described somewhere in the filesystem, or there'd be
> a syscall/ioctl/whatever to go from an interface to the appropriate
> monitoring interface.
>
> That all depends on exactly how this gets exposed in the end, however.
>

After the first version of the patch was submitted, the feedback from 
the netdev list was to expose it as a network interface as this would
allow it to be accessed by standard user space monitoring tools.
It definitely makes sense to link it to the associated ethernet netdev,
but I am not sure if there is a framework in the kernel to do it at
the driver level?

> Thanks,
> Mark.
> --

Thank you,
Stathis

Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix

[Mark Rutland]

On Wed, Feb 18, 2015 at 09:40:22AM +0000, Stathis Voukelatos wrote:
> 
> 
> On 17/02/15 17:30, Mark Rutland wrote:
> 
> >> It is the frequency of the timestamp values supplied to the sniffer
> >> module. It is used by the driver to convert to nanoseconds.
> >> I was trying to be somewhat generic here and not assume that it
> >> is necessarily the same as the 'tstamp' clock below, in which case we
> >> could indeed obtain it using the common clock framework.
> >
> > In what cases would it _not_ be the same? From your description this is
> > that clock, no?
> >
> 
> Counters can often have a divider applied to their input clock and
> therefore run at a scaled down frequency. This is not the case in the
> first SoC where the sniffer is used, so for simplicity I can modify as
> you suggest and remove that field from the DT.

The common clock bindings have fixed-factor-clock for handling dividers,
so I believe you should be able to use that.

You mentioned that the counter was a block external to the sniffer. Does
it have any configuration interface (e.g. to reset the counter)? We may
need to model it in the DT if so (and describe the clock as feeding into
it rather than into the sniffer).

> >> Yes, but the sniffer module is hard-wired to a certain Ethernet Mii
> >> interface. We can add an entry to tie it to an Ethernet controller, but
> >> apart of a sanity check I am not sure what else the S/W can do.
> >
> > Fundamentally, the use-case for this is monitoring an ethernet
> > interface. So regardless of which kernel framework this plumbs into,
> > there needs to be a way to go from ethN to whatever this is exposed as.
> >
> > Exposing a completely separate interface makes no sense. Singleton stuff
> > like that inevitably gets broken as someone later builds a board with
> > multiple instances of some similar IP block.
> >
> > So I would imagine that either the link between interface and monitoring
> > interface would be described somewhere in the filesystem, or there'd be
> > a syscall/ioctl/whatever to go from an interface to the appropriate
> > monitoring interface.
> >
> > That all depends on exactly how this gets exposed in the end, however.
> >
> 
> After the first version of the patch was submitted, the feedback from 
> the netdev list was to expose it as a network interface as this would
> allow it to be accessed by standard user space monitoring tools.
> It definitely makes sense to link it to the associated ethernet netdev,
> but I am not sure if there is a framework in the kernel to do it at
> the driver level?

Unfortunately I'm not familiar enough with the net code; hopefully
someone else can point us in the right direction.

Thanks,
Mark.

Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix

[Stathis Voukelatos]

Hi Mark,

On 18/02/15 12:11, Mark Rutland wrote:
>>
>> Counters can often have a divider applied to their input clock and
>> therefore run at a scaled down frequency. This is not the case in the
>> first SoC where the sniffer is used, so for simplicity I can modify as
>> you suggest and remove that field from the DT.
>
> The common clock bindings have fixed-factor-clock for handling dividers,
> so I believe you should be able to use that.
>
> You mentioned that the counter was a block external to the sniffer. Does
> it have any configuration interface (e.g. to reset the counter)? We may
> need to model it in the DT if so (and describe the clock as feeding into
> it rather than into the sniffer).
>

The sniffer module is designed so that it receives a Gray encoded
timestamp from another module of the SoC that it is integrated in.
The first integration that we have is in the IMG Pistachio SoC.
There the timestamp counter is part of the Event Timer module
and its source is one of the system clocks of the chip (configurable)
That module will eventually have it's own device-tree node when a
driver becomes available for it.

The 'tstamp' clock given in the sniffer DT node should be such
that when it is enabled it will start the counter and its parent clock,
so that it can start counting.

> Thanks,
> Mark.
>

Stathis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/3] Packet sniffer core framework

[Stathis Voukelatos]

The framework registers each backend sniffer channel as a netdev,
which can be accessed from user space through a raw packet socket.
Packets received from user space are treated as a command string
configuration. Each match event from the backend driver will
generate a packet with the matching bytes plus an optional
timestamp, if configured by the command string.

Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
---
 MAINTAINERS                             |   6 +
 drivers/net/Kconfig                     |   2 +
 drivers/net/Makefile                    |   2 +
 drivers/net/pkt-sniffer/Kconfig         |   8 +
 drivers/net/pkt-sniffer/Makefile        |   3 +
 drivers/net/pkt-sniffer/core/module.c   |  37 +++++
 drivers/net/pkt-sniffer/core/netdev.c   | 254 ++++++++++++++++++++++++++++++++
 drivers/net/pkt-sniffer/core/snf_core.h |  60 ++++++++
 include/uapi/linux/pkt_sniffer.h        |  33 +++++
 9 files changed, 405 insertions(+)
 create mode 100644 drivers/net/pkt-sniffer/Kconfig
 create mode 100644 drivers/net/pkt-sniffer/Makefile
 create mode 100644 drivers/net/pkt-sniffer/core/module.c
 create mode 100644 drivers/net/pkt-sniffer/core/netdev.c
 create mode 100644 drivers/net/pkt-sniffer/core/snf_core.h
 create mode 100644 include/uapi/linux/pkt_sniffer.h

diff --git a/MAINTAINERS b/MAINTAINERS
index aaa039d..7d882de 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5754,6 +5754,12 @@ M:	Sasha Levin <sasha.levin@oracle.com>
 S:	Maintained
 F:	tools/lib/lockdep/
 
+LINN PACKET SNIFFER DRIVER
+M: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
+S: Maintained
+F: drivers/net/pkt-sniffer/
+F: Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
+
 LINUX FOR IBM pSERIES (RS/6000)
 M:	Paul Mackerras <paulus@au.ibm.com>
 W:	http://www.ibm.com/linux/ltc/projects/ppc
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index d6607ee..219c786 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -380,4 +380,6 @@ config VMXNET3
 
 source "drivers/net/hyperv/Kconfig"
 
+source "drivers/net/pkt-sniffer/Kconfig"
+
 endif # NETDEVICES
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index e25fdd7..56ed84e 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -66,3 +66,5 @@ obj-$(CONFIG_USB_NET_DRIVERS) += usb/
 
 obj-$(CONFIG_HYPERV_NET) += hyperv/
 obj-$(CONFIG_NTB_NETDEV) += ntb_netdev.o
+obj-$(CONFIG_PKT_SNIFFER) += pkt-sniffer/
+
diff --git a/drivers/net/pkt-sniffer/Kconfig b/drivers/net/pkt-sniffer/Kconfig
new file mode 100644
index 0000000..53ffcc1
--- /dev/null
+++ b/drivers/net/pkt-sniffer/Kconfig
@@ -0,0 +1,8 @@
+menuconfig PKT_SNIFFER
+    tristate "Packet sniffer support"
+    ---help---
+    Say Y to add support for the packet sniffer driver framework.
+
+    The core driver can also be built as a module. If so, the module
+    will be called snf_core.
+
diff --git a/drivers/net/pkt-sniffer/Makefile b/drivers/net/pkt-sniffer/Makefile
new file mode 100644
index 0000000..31dc396
--- /dev/null
+++ b/drivers/net/pkt-sniffer/Makefile
@@ -0,0 +1,3 @@
+snf_core-y += core/netdev.o
+snf_core-y += core/module.o
+obj-$(CONFIG_PKT_SNIFFER) += snf_core.o
diff --git a/drivers/net/pkt-sniffer/core/module.c b/drivers/net/pkt-sniffer/core/module.c
new file mode 100644
index 0000000..1dbed1f
--- /dev/null
+++ b/drivers/net/pkt-sniffer/core/module.c
@@ -0,0 +1,37 @@
+/*
+ * Packet sniffer core driver:
+ *  - backend channel management
+ *  - interface to userland as a network I/F
+ *
+ * Copyright (C) 2015 Linn Products Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Written by:
+ * Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
+ */
+#include <linux/module.h>
+#include <linux/init.h>
+
+static int __init snf_core_init(void)
+{
+	return 0;
+}
+
+static void __exit snf_core_cleanup(void)
+{
+}
+
+module_init(snf_core_init);
+module_exit(snf_core_cleanup);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Core packet sniffer driver");
+MODULE_AUTHOR("Linn Products Ltd");
diff --git a/drivers/net/pkt-sniffer/core/netdev.c b/drivers/net/pkt-sniffer/core/netdev.c
new file mode 100644
index 0000000..ba25cc0
--- /dev/null
+++ b/drivers/net/pkt-sniffer/core/netdev.c
@@ -0,0 +1,254 @@
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+#include <linux/net_tstamp.h>
+#include <linux/if_arp.h>
+#include <linux/spinlock.h>
+#include "snf_core.h"
+
+struct snf_ndev_state {
+	struct snf_chan *chan;
+	bool rx_tstamp_enabled;
+	spinlock_t lock;
+};
+
+static int hw_timestamp_set(struct net_device *dev, struct ifreq *ifr)
+{
+	struct snf_ndev_state *priv = netdev_priv(dev);
+	struct hwtstamp_config tconf;
+
+	if (copy_from_user(&tconf, ifr->ifr_data, sizeof(tconf)))
+		return -EFAULT;
+
+	/* No TX timestamping supported.
+	 * This interface only receives packets from the sniffer backend
+	 */
+	if (tconf.tx_type != HWTSTAMP_TX_OFF)
+		return -ERANGE;
+
+	if (tconf.rx_filter != HWTSTAMP_FILTER_NONE) {
+		/* If timestamping is not enabled in the command string then
+		 * we cannot return any RX timestamps
+		 */
+		if (!priv->chan->ts_enabled(priv->chan))
+			return -ERANGE;
+		priv->rx_tstamp_enabled = true;
+		tconf.rx_filter = HWTSTAMP_FILTER_ALL;
+	} else {
+		priv->rx_tstamp_enabled = false;
+	}
+
+	return copy_to_user(ifr->ifr_data, &tconf, sizeof(tconf)) ? -EFAULT : 0;
+}
+
+static int hw_timestamp_get(struct net_device *dev, struct ifreq *ifr)
+{
+	struct snf_ndev_state *priv = netdev_priv(dev);
+	struct hwtstamp_config tconf;
+
+	memset(&tconf, 0, sizeof(tconf));
+	tconf.tx_type = HWTSTAMP_TX_OFF;
+	/* We also need to check here that the current command string
+	 * will cause timestamps to be generated
+	 */
+	tconf.rx_filter = (priv->rx_tstamp_enabled &&
+			   priv->chan->ts_enabled(priv->chan)) ?
+			   HWTSTAMP_FILTER_ALL : HWTSTAMP_FILTER_NONE;
+
+	return copy_to_user(ifr->ifr_data, &tconf, sizeof(tconf)) ? -EFAULT : 0;
+}
+
+static int snf_init(struct net_device *dev)
+{
+	struct snf_ndev_state *priv = netdev_priv(dev);
+
+	/* Two bytes per command string entry */
+	dev->mtu = priv->chan->max_ptn_entries(priv->chan) * 2;
+	return 0;
+}
+
+static int snf_open(struct net_device *dev)
+{
+	struct snf_ndev_state *priv = netdev_priv(dev);
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	ret = priv->chan->start(priv->chan);
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return ret;
+}
+
+static int snf_stop(struct net_device *dev)
+{
+	struct snf_ndev_state *priv = netdev_priv(dev);
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	ret = priv->chan->stop(priv->chan);
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return ret;
+}
+
+static int snf_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
+{
+	switch (cmd) {
+	case SIOCSHWTSTAMP:
+		return hw_timestamp_set(dev, ifr);
+
+	case SIOCGHWTSTAMP:
+		return hw_timestamp_get(dev, ifr);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int snf_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct snf_ndev_state *priv = netdev_priv(dev);
+	struct snf_chan *sch = priv->chan;
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	/* Stop the hardware */
+	sch->stop(sch);
+	/* Set the new command pattern */
+	ret = sch->set_pattern(sch, skb->data, skb->len / 2);
+	/* Restart the hardware */
+	sch->start(sch);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	if (ret < 0) {
+		dev->stats.tx_dropped++;
+	} else {
+		dev->stats.tx_packets++;
+		dev->stats.tx_bytes += skb->len;
+	}
+
+	dev_kfree_skb(skb);
+	return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops snf_netdev_ops = {
+	.ndo_init       = snf_init,
+	.ndo_open       = snf_open,
+	.ndo_stop       = snf_stop,
+	.ndo_start_xmit	= snf_start_xmit,
+	.ndo_do_ioctl   = snf_ioctl
+};
+
+static void snf_setup(struct net_device *dev)
+{
+	dev->netdev_ops   = &snf_netdev_ops;
+	dev->tx_queue_len = 1;
+	dev->flags        = IFF_NOARP;
+	dev->type         = ARPHRD_NONE;
+}
+
+/* Initialise netdev for a sniffer channel */
+int snf_channel_add(struct snf_chan *sch, const char *name)
+{
+	int ret;
+	struct net_device *ndev;
+	struct snf_ndev_state *priv;
+
+	ndev = alloc_netdev(sizeof(*priv), name, NET_NAME_UNKNOWN, snf_setup);
+	if (!ndev)
+		return -ENOMEM;
+
+	priv = netdev_priv(ndev);
+	priv->chan = sch;
+	priv->rx_tstamp_enabled = false;
+	spin_lock_init(&priv->lock);
+
+	ret = register_netdev(ndev);
+	if (ret < 0) {
+		free_netdev(ndev);
+		return ret;
+	}
+
+	sch->cstate = ndev;
+	return 0;
+}
+EXPORT_SYMBOL(snf_channel_add);
+
+/* Release netdev for a sniffer channel and free resources */
+int snf_channel_remove(struct snf_chan *sch)
+{
+	struct net_device *ndev = (struct net_device *)sch->cstate;
+
+	unregister_netdev(ndev);
+	free_netdev(ndev);
+	return 0;
+}
+EXPORT_SYMBOL(snf_channel_remove);
+
+/* Send a packet to user space for a sniffer match event */
+int snf_channel_notify_match(struct snf_chan *sch, struct snf_match_evt *mt)
+{
+	struct net_device *ndev = (struct net_device *)sch->cstate;
+	struct snf_ndev_state *priv = netdev_priv(ndev);
+	struct sk_buff *skb;
+	struct skb_shared_hwtstamps *skts;
+
+	skb = netdev_alloc_skb(ndev, mt->len);
+	if (!skb) {
+		ndev->stats.rx_dropped++;
+		return -ENOMEM;
+	}
+
+	skb_put(skb, mt->len);
+	skb_copy_to_linear_data(skb, mt->data, mt->len);
+	if (mt->ts_valid && priv->rx_tstamp_enabled) {
+		skts = skb_hwtstamps(skb);
+		skts->hwtstamp = ns_to_ktime(mt->ts);
+	}
+	ndev->stats.rx_packets++;
+	ndev->stats.rx_bytes += mt->len;
+	netif_rx(skb);
+	return 0;
+}
+EXPORT_SYMBOL(snf_channel_notify_match);
+
+/* Suspend the interface */
+int snf_channel_suspend(struct snf_chan *sch)
+{
+	struct net_device *ndev = (struct net_device *)sch->cstate;
+	struct snf_ndev_state *priv = netdev_priv(ndev);
+	unsigned long flags;
+	int ret;
+
+	if (!netif_running(ndev))
+		return 0;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	ret = sch->stop(sch);
+	spin_unlock_irqrestore(&priv->lock, flags);
+	netif_device_detach(ndev);
+	return ret;
+}
+EXPORT_SYMBOL(snf_channel_suspend);
+
+/* Resume the interface */
+int snf_channel_resume(struct snf_chan *sch)
+{
+	struct net_device *ndev = (struct net_device *)sch->cstate;
+	struct snf_ndev_state *priv = netdev_priv(ndev);
+	unsigned long flags;
+	int ret;
+
+	if (!netif_running(ndev))
+		return 0;
+
+	netif_device_attach(ndev);
+	spin_lock_irqsave(&priv->lock, flags);
+	ret = sch->start(sch);
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return ret;
+}
+EXPORT_SYMBOL(snf_channel_resume);
+
diff --git a/drivers/net/pkt-sniffer/core/snf_core.h b/drivers/net/pkt-sniffer/core/snf_core.h
new file mode 100644
index 0000000..73a30ff
--- /dev/null
+++ b/drivers/net/pkt-sniffer/core/snf_core.h
@@ -0,0 +1,60 @@
+/*
+ * Packet sniffer core driver
+ * - this header provides the interface to specific backend packet
+ *   sniffer implementations
+ *
+ * Copyright (C) 2015 Linn Products Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Written by:
+ * Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
+ */
+#ifndef __SNF_CORE_H
+#define __SNF_CORE_H
+
+#include <linux/types.h>
+
+/* This is a global maximum. Each backend will have its own limit */
+#define MAX_MATCH_BYTES 512
+
+/* Sniffer channel data structure */
+struct snf_chan {
+	int  (*start)(struct snf_chan *sch);
+	int  (*stop)(struct snf_chan *sch);
+	int  (*set_pattern)(struct snf_chan *sch, const u8 *pattern, int count);
+	int  (*max_ptn_entries)(struct snf_chan *sch);
+	int  (*ts_enabled)(struct snf_chan *sch);
+	void *cstate;
+};
+
+/* Data from a sniffer match event */
+struct snf_match_evt {
+	int ts_valid;     /* flag indicating if timestamp is present */
+	u64 ts;           /* timestamp value */
+	u8 data[MAX_MATCH_BYTES]; /* packet data bytes matched by sniffer */
+	int len;          /* number of valid bytes in the 'data' buffer */
+};
+
+/* Registers a sniffer channel */
+int snf_channel_add(struct snf_chan *sch, const char *name);
+
+/* Removes a sniffer channel */
+int snf_channel_remove(struct snf_chan *sch);
+
+/* Send a packet to user space for a sniffer match event */
+int snf_channel_notify_match(struct snf_chan *sch, struct snf_match_evt *mt);
+
+/* Suspend and resume operations */
+int snf_channel_suspend(struct snf_chan *sch);
+int snf_channel_resume(struct snf_chan *sch);
+
+#endif
+
diff --git a/include/uapi/linux/pkt_sniffer.h b/include/uapi/linux/pkt_sniffer.h
new file mode 100644
index 0000000..39e9957
--- /dev/null
+++ b/include/uapi/linux/pkt_sniffer.h
@@ -0,0 +1,33 @@
+/*
+ * Linn packet sniffer driver interface
+ *
+ * Copyright (C) 2015 Linn Products Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Written by:
+ * Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
+ */
+#ifndef __PKT_SNIFFER_H
+#define __PKT_SNIFFER_H
+
+/* Commands for the command string
+ * It consists of a series of bytes in the following format:
+ *  --------------------------------
+ *  | CMD | DATA | CMD | DATA | ....
+ *  --------------------------------
+ */
+#define PTN_CMD_DONTCARE	0
+#define PTN_CMD_MATCH		1
+#define PTN_CMD_COPY		2
+#define PTN_CMD_MATCHSTAMP	3
+#define PTN_CMD_COPYDONE	4
+
+#endif
-- 
1.9.1

Re: [PATCH v2 2/3] Packet sniffer core framework

[Daniel Borkmann]

On 02/17/2015 03:03 PM, Stathis Voukelatos wrote:
> The framework registers each backend sniffer channel as a netdev,
> which can be accessed from user space through a raw packet socket.
> Packets received from user space are treated as a command string
> configuration. Each match event from the backend driver will
> generate a packet with the matching bytes plus an optional
> timestamp, if configured by the command string.
>
> Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
> ---
>   MAINTAINERS                             |   6 +
>   drivers/net/Kconfig                     |   2 +
>   drivers/net/Makefile                    |   2 +
>   drivers/net/pkt-sniffer/Kconfig         |   8 +
>   drivers/net/pkt-sniffer/Makefile        |   3 +
>   drivers/net/pkt-sniffer/core/module.c   |  37 +++++
>   drivers/net/pkt-sniffer/core/netdev.c   | 254 ++++++++++++++++++++++++++++++++
>   drivers/net/pkt-sniffer/core/snf_core.h |  60 ++++++++
>   include/uapi/linux/pkt_sniffer.h        |  33 +++++
>   9 files changed, 405 insertions(+)
>   create mode 100644 drivers/net/pkt-sniffer/Kconfig
>   create mode 100644 drivers/net/pkt-sniffer/Makefile
>   create mode 100644 drivers/net/pkt-sniffer/core/module.c
>   create mode 100644 drivers/net/pkt-sniffer/core/netdev.c
>   create mode 100644 drivers/net/pkt-sniffer/core/snf_core.h
>   create mode 100644 include/uapi/linux/pkt_sniffer.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aaa039d..7d882de 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5754,6 +5754,12 @@ M:	Sasha Levin <sasha.levin@oracle.com>
>   S:	Maintained
>   F:	tools/lib/lockdep/
>
> +LINN PACKET SNIFFER DRIVER
> +M: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
> +S: Maintained
> +F: drivers/net/pkt-sniffer/
> +F: Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt

This whole framework really looks like only tailored to your specific
driver, I have no idea who else should reuse that?! So, I don't think
putting this under drivers/net/pkt-sniffer/ is a good idea.

Also it looks slightly confusing as if I understand you correctly, your
module's purpose is to pass down some "packet pattern" to the hardware
and match that in order to get a precise timestamp in return?

Might perhaps be better to have everything vendor-specific under something
like drivers/net/ethernet/linn/ and have the framework squashed into the
driver itself (if parts cannot be generalized in net/packet/).

It would be good if you can also avoid the extra uapi export. Perhaps
it's possible to reuse at least some of the existing timestamping
infrastructure?

Re: [PATCH v2 2/3] Packet sniffer core framework

[Stathis Voukelatos]

Hi Daniel,

On 18/02/15 15:42, Daniel Borkmann wrote:
>
> This whole framework really looks like only tailored to your specific
> driver, I have no idea who else should reuse that?! So, I don't think
> putting this under drivers/net/pkt-sniffer/ is a good idea.
>

Yes, it is not necessarilly expected to be used by other 3rd party 
drivers. The reason of splitting out the framework code is to account of 
the fact the we may develop in the future othersimilar sniffer H/W for 
non-ethernet interfaces (eg. wifi).
I can move the code under drivers/net/ethernet/linn as you mention 
below, although that may not account for non-ethernet backends in the
future.

> Also it looks slightly confusing as if I understand you correctly, your
> module's purpose is to pass down some "packet pattern" to the hardware
> and match that in order to get a precise timestamp in return?
>

Yes, this point can be slightly confusing. A write to a packet socket 
bound to the interface is done to supply the command string to the 
sniffer H/W, while reads would return matched packet bytes + timestamps 
(throuch cmsg). Is there any other way to supply the command string 
except of a proprietary ioctl?

> Might perhaps be better to have everything vendor-specific under something
> like drivers/net/ethernet/linn/ and have the framework squashed into the
> driver itself (if parts cannot be generalized in net/packet/).
>

Answered above.

> It would be good if you can also avoid the extra uapi export. Perhaps
> it's possible to reuse at least some of the existing timestamping
> infrastructure?

I can remove that. The header file only contains the list of commands.
They can be documented. The driver does use the existing timestamping
infrastructure to return timestamps to user space.

Thank you,
Stathis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/3] Linn Ethernet packet sniffer driver

[Stathis Voukelatos]

Driver for the Ethernet Mii packet sniffer H/W module found in
the IMG Pistachio SoC.

Signed-off-by: Stathis Voukelatos <stathis.voukelatos-zgcZaY4qg+21Qrn1Bg8BZw@public.gmane.org>
---
 drivers/net/pkt-sniffer/Kconfig                   |  11 +
 drivers/net/pkt-sniffer/Makefile                  |   4 +
 drivers/net/pkt-sniffer/backends/ether/channel.c  | 392 ++++++++++++++++++++++
 drivers/net/pkt-sniffer/backends/ether/channel.h  |  81 +++++
 drivers/net/pkt-sniffer/backends/ether/hw.h       |  46 +++
 drivers/net/pkt-sniffer/backends/ether/platform.c | 301 +++++++++++++++++
 6 files changed, 835 insertions(+)
 create mode 100644 drivers/net/pkt-sniffer/backends/ether/channel.c
 create mode 100644 drivers/net/pkt-sniffer/backends/ether/channel.h
 create mode 100644 drivers/net/pkt-sniffer/backends/ether/hw.h
 create mode 100644 drivers/net/pkt-sniffer/backends/ether/platform.c

diff --git a/drivers/net/pkt-sniffer/Kconfig b/drivers/net/pkt-sniffer/Kconfig
index 53ffcc1..b7c7f6b 100644
--- a/drivers/net/pkt-sniffer/Kconfig
+++ b/drivers/net/pkt-sniffer/Kconfig
@@ -6,3 +6,14 @@ menuconfig PKT_SNIFFER
     The core driver can also be built as a module. If so, the module
     will be called snf_core.
 
+config PKT_SNIFFER_ETHER
+    tristate "Ethernet packet sniffer"
+    depends on PKT_SNIFFER
+    help
+        Say Y here if you want to use the Ethernet packet sniffer
+        module by Linn Products Ltd. It can be found in the upcoming
+        Pistachio SoC by Imagination Technologies.
+
+        The driver can also be built as a module. If so, the module
+        will be called snf_ether.
+
diff --git a/drivers/net/pkt-sniffer/Makefile b/drivers/net/pkt-sniffer/Makefile
index 31dc396..89a3c60 100644
--- a/drivers/net/pkt-sniffer/Makefile
+++ b/drivers/net/pkt-sniffer/Makefile
@@ -1,3 +1,7 @@
 snf_core-y += core/netdev.o
 snf_core-y += core/module.o
 obj-$(CONFIG_PKT_SNIFFER) += snf_core.o
+
+snf_ether-y += backends/ether/platform.o
+snf_ether-y += backends/ether/channel.o
+obj-$(CONFIG_PKT_SNIFFER_ETHER) += snf_ether.o
diff --git a/drivers/net/pkt-sniffer/backends/ether/channel.c b/drivers/net/pkt-sniffer/backends/ether/channel.c
new file mode 100644
index 0000000..a7fed4e
--- /dev/null
+++ b/drivers/net/pkt-sniffer/backends/ether/channel.c
@@ -0,0 +1,392 @@
+/*
+ * Ethernet Mii packet sniffer driver
+ *  - channel functions
+ *
+ * Copyright (C) 2015 Linn Products Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Written by:
+ * Stathis Voukelatos <stathis.voukelatos-zgcZaY4qg+21Qrn1Bg8BZw@public.gmane.org>
+ */
+#include <linux/io.h>
+#include <linux/hrtimer.h>
+#include <linux/pkt_sniffer.h>
+#include "../../core/snf_core.h"
+#include "hw.h"
+#include "channel.h"
+
+#define to_ether_snf_chan(dev) container_of(dev, struct ether_snf_chan, chan)
+
+/* Checks if the supplied command string is compatible with the
+ * capabilities of the H/W. The command string consists of a series
+ * of bytes in the following format
+ *  --------------------------------
+ *  | CMD | DATA | CMD | DATA | ....
+ *  --------------------------------
+ */
+static bool validate_pattern(
+			struct ether_snf_chan *ch,
+			const u8 *buf,
+			int count)
+{
+	int i, complete, max_copy_bytes;
+	int ts = 0;
+	int copy_before = 0;
+	int copy_after = 0;
+
+	if (count > ch->max_cmds)
+		return false;
+
+	/* Iterate through the commands in the string */
+	for (i = 0, complete = 0; (i < count) && !complete; i++) {
+		u8 cmd = buf[2*i];
+
+		switch (cmd) {
+		case PTN_CMD_DONTCARE:
+		case PTN_CMD_MATCH:
+			break;
+
+		case PTN_CMD_MATCHSTAMP:
+			/* The timestamp needs to be word-aligned in the FIFO
+			 * therefore, the number of 'copy' commands before it
+			 * needs to be a multiple of 4
+			 */
+			if (copy_before & 3)
+				return false;
+			/* Signal that a timestamp will be present */
+			ts = 1;
+			break;
+
+		case PTN_CMD_COPY:
+			/* Increment count of bytes that will be returned */
+			if (ts)
+				copy_after++;
+			else
+				copy_before++;
+			break;
+
+		case PTN_CMD_COPYDONE:
+			/* Increment count of bytes that will be returned
+			 * This command terminates the string
+			 */
+			if (ts)
+				copy_after++;
+			else
+				copy_before++;
+			/* This command completes the command string */
+			complete = 1;
+			break;
+
+		default:
+			/* Invalid command id */
+			return false;
+		}
+	}
+
+	/* Check if the string was properly terminated
+	 * and contained valid number of commands
+	 */
+	if (complete) {
+		max_copy_bytes = ch->fifo_blk_words * 4;
+		if (ts)
+			max_copy_bytes -= 4;
+		/* Too many copy commands will case the FIFO
+		 * to wrap around
+		 */
+		if ((copy_before + copy_after) > max_copy_bytes)
+			return false;
+		ch->nfb_before = copy_before;
+		ch->nfb_after = copy_after;
+		ch->evt.ts_valid = ts;
+		ch->evt.len = ch->nfb_before + ch->nfb_after;
+		return true;
+	}
+
+	/* Command string not terminated */
+	return false;
+}
+
+/* Channel methods */
+
+/* Enables the channel */
+static int esnf_start(struct snf_chan *dev)
+{
+	struct ether_snf_chan *ch = to_ether_snf_chan(dev);
+
+	if (!ch->started) {
+		ch->evt.ts = 0;
+		/* Enable interrupts */
+		iowrite32(ch->data_irq_bit | ch->full_irq_bit,
+			  ch->regs + SET_INTERRUPT_ENABLE);
+		/* Enable the packet matching logic */
+		iowrite32(ENABLE_BIT, ch->reg_enable);
+
+		ch->started = 1;
+		dev_info(ch->dev, "%s: started\n", ch->name);
+	} else {
+		dev_dbg(ch->dev, "%s: already running\n", ch->name);
+	}
+
+	return 0;
+}
+
+/* Disables the channel */
+static int esnf_stop(struct snf_chan *dev)
+{
+	struct ether_snf_chan *ch = to_ether_snf_chan(dev);
+
+	if (ch->started) {
+		/* Disable interrupts */
+		iowrite32(ch->data_irq_bit | ch->full_irq_bit,
+			  ch->regs + CLEAR_INTERRUPT_ENABLE);
+		/* Stop the sniffer channel */
+		iowrite32(0, ch->reg_enable);
+		/* Clear any pending interrupts */
+		iowrite32(ch->data_irq_bit | ch->full_irq_bit,
+			  ch->regs + INTERRUPT_STATUS);
+
+		ch->started = 0;
+		dev_info(ch->dev, "%s: stopped\n", ch->name);
+	} else {
+		dev_dbg(ch->dev, "%s: already stopped\n", ch->name);
+	}
+
+	return 0;
+}
+
+/* Sets the command string (pattern) for the channel
+ * The bytes in the pattern buffer are in the following order:
+ */
+static int esnf_set_pattern(struct snf_chan *dev, const u8 *pattern, int count)
+{
+	struct ether_snf_chan *ch = to_ether_snf_chan(dev);
+	int i, shift = 0;
+	u32  val = 0, *ptr;
+
+	dev_info(ch->dev, "%s: set cmd pattern with %d entries\n",
+		 ch->name, count);
+
+	if (ch->started) {
+		dev_err(ch->dev,
+			"%s: cannot apply cmd pattern. Channel is active\n",
+			ch->name);
+		return -EBUSY;
+	}
+
+	if (!validate_pattern(ch, pattern, count)) {
+		dev_err(ch->dev,
+			"%s: invalid cmd pattern\n",
+			ch->name);
+		return -EINVAL;
+	}
+
+	for (ptr = ch->cmd_ram, i = 0, shift = 24; i < (2*count); i++) {
+		val |= ((u32)pattern[i]) << shift;
+		if (!shift) {
+			iowrite32(val, ptr++);
+			val = 0;
+			shift = 24;
+		} else {
+			shift -= 8;
+		}
+	}
+	if (shift)
+		iowrite32(val, ptr);
+
+	return 0;
+}
+
+/* Returns max number of commands supported by the channel */
+static int esnf_max_ptn_entries(struct snf_chan *dev)
+{
+	struct ether_snf_chan *ch = to_ether_snf_chan(dev);
+
+	return ch->max_cmds;
+}
+
+static int esnf_ts_enabled(struct snf_chan *dev)
+{
+	struct ether_snf_chan *ch = to_ether_snf_chan(dev);
+
+	return ch->evt.ts_valid;
+}
+
+/* Gray decoder */
+static u32 gray_decode(u32 gray)
+{
+	gray ^= (gray >> 16);
+	gray ^= (gray >> 8);
+	gray ^= (gray >> 4);
+	gray ^= (gray >> 2);
+	gray ^= (gray >> 1);
+	return gray;
+}
+
+/* Read a block from the data FIFO */
+static void read_fifo_data(struct ether_snf_chan *ch)
+{
+	int i;
+	u32 val, *ptr;
+	int ts = ch->evt.ts_valid;
+	int dw = ch->fifo_blk_words;
+	int bytes_before = ch->nfb_before;
+	int bytes_after = ch->nfb_after;
+
+	ptr = (u32 *)ch->evt.data;
+	for (i = 0; i < dw; i++) {
+		val = ioread32(ch->reg_fifo);
+		if (bytes_before > 0) {
+			/* Bytes before the timestamp */
+			*ptr++ = cpu_to_be32(val);
+			bytes_before -= 4;
+		} else if (ts) {
+			ch->raw_tstamp = gray_decode(val);
+			/* First timestamp since the channel was started.
+			 * Initialise the timecounter
+			 */
+			if (!ch->evt.ts)
+				timecounter_init(
+					&ch->tc,
+					&ch->cc,
+					ktime_to_ns(ktime_get_real()));
+			ch->evt.ts = timecounter_read(&ch->tc);
+			ts = 0;
+		} else if (bytes_after > 0) {
+			/* Bytes after the timestamp */
+			*ptr++ = cpu_to_be32(val);
+			bytes_after -= 4;
+		}
+	}
+}
+
+/* Read method for the timestamp cycle counter
+ * Returns the last received timestamp value
+ */
+static cycle_t esnf_cyclecounter_read(const struct cyclecounter *cc)
+{
+	struct ether_snf_chan *ch = container_of(cc, struct ether_snf_chan, cc);
+
+	return ch->raw_tstamp;
+}
+
+/* Initialises a sniffer channel */
+int channel_init(
+	struct ether_snf_chan *ch,
+	struct platform_device *pdev,
+	void *regs,
+	int fifo_blk_words,
+	u32 tstamp_hz,
+	u32 tstamp_shift,
+	u32 tstamp_bits,
+	int tx)
+{
+	struct resource *res;
+	u32 *ptr;
+	int i;
+
+	ch->regs = regs;
+	ch->dev = &pdev->dev;
+	ch->data_irq_bit = tx ? TX_DATA_IRQ_BIT : RX_DATA_IRQ_BIT;
+	ch->full_irq_bit = tx ? TX_FULL_IRQ_BIT : RX_FULL_IRQ_BIT;
+	ch->reg_enable = ch->regs +
+			 (tx ? TX_SNIFFER_ENABLE : RX_SNIFFER_ENABLE);
+	ch->reg_fifo = ch->regs + (tx ? TX_FIFO_DAT : RX_FIFO_DAT);
+
+	/* Retrieve and remap the address space for the command memory */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+					   tx ? "tx-ram" : "rx-ram");
+	if (!res)
+		return -ENOSYS;
+	ch->cmd_ram = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ch->cmd_ram))
+		return PTR_ERR(ch->cmd_ram);
+
+	/* It is 2 bytes/command, hence divide by 2 */
+	ch->max_cmds = resource_size(res) / 2;
+
+	/* Initialise the command string RAM */
+	for (i = 0, ptr = ch->cmd_ram; i < resource_size(res); i += 4)
+		iowrite32((PTN_CMD_DONTCARE << 24) | (PTN_CMD_DONTCARE << 8),
+			  ptr++);
+
+	ch->fifo_blk_words = fifo_blk_words;
+	ch->started = 0;
+
+	/* Initialise the timestamp cycle counter */
+	ch->cc.read = esnf_cyclecounter_read;
+	ch->cc.mask = CLOCKSOURCE_MASK(tstamp_bits);
+	ch->cc.mult = clocksource_hz2mult(tstamp_hz, tstamp_shift);
+	ch->cc.shift = tstamp_shift;
+
+	/* Register the channel methods */
+	ch->chan.start = esnf_start;
+	ch->chan.stop = esnf_stop;
+	ch->chan.set_pattern = esnf_set_pattern;
+	ch->chan.max_ptn_entries = esnf_max_ptn_entries;
+	ch->chan.ts_enabled = esnf_ts_enabled;
+
+	strncpy(ch->name, tx ? "TX" : "RX", MAX_CHAN_NAME_SIZE - 1);
+
+	dev_dbg(ch->dev, "%s: channel initialized\n", ch->name);
+
+	return 0;
+}
+
+/* Registers the channel with the sniffer core module */
+int channel_register(struct ether_snf_chan *ch, const char *name)
+{
+	int ret;
+
+	ret = snf_channel_add(&ch->chan, name);
+	if (ret < 0)
+		return ret;
+
+	dev_info(ch->dev, "%s channel added\n", ch->name);
+	return 0;
+}
+
+/* Unregisters the channel */
+int channel_unregister(struct ether_snf_chan *ch)
+{
+	int ret;
+
+	ch->chan.stop(&ch->chan);
+	ret = snf_channel_remove(&ch->chan);
+	if (!ret)
+		dev_info(ch->dev, "%s channel removed\n", ch->name);
+	return ret;
+}
+
+/* Process match event data */
+void channel_data_available(struct ether_snf_chan *ch)
+{
+	int ret;
+
+	dev_dbg(ch->dev, "%s match event\n", ch->name);
+
+	read_fifo_data(ch);
+	ret = snf_channel_notify_match(&ch->chan, &ch->evt);
+	if (ret < 0)
+		dev_err(ch->dev, "%s: event notification failed\n", ch->name);
+}
+
+/* Suspend opertion */
+int channel_suspend(struct ether_snf_chan *ch)
+{
+	return snf_channel_suspend(&ch->chan);
+}
+
+/* Resume operation */
+int channel_resume(struct ether_snf_chan *ch)
+{
+	return snf_channel_resume(&ch->chan);
+}
+
diff --git a/drivers/net/pkt-sniffer/backends/ether/channel.h b/drivers/net/pkt-sniffer/backends/ether/channel.h
new file mode 100644
index 0000000..35ed891
--- /dev/null
+++ b/drivers/net/pkt-sniffer/backends/ether/channel.h
@@ -0,0 +1,81 @@
+/*
+ * Ethernet Mii packet sniffer driver
+ *  - channel interface
+ *
+ * Copyright (C) 2015 Linn Products Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Written by:
+ * Stathis Voukelatos <stathis.voukelatos-zgcZaY4qg+21Qrn1Bg8BZw@public.gmane.org>
+ */
+#ifndef _ETHER_SNIFFER_CHANNEL_H_
+#define _ETHER_SNIFFER_CHANNEL_H_
+
+#include <linux/platform_device.h>
+#include <linux/clocksource.h>
+#include "../../core/snf_core.h"
+
+#define MAX_CHAN_NAME_SIZE 5
+
+struct ether_snf_chan {
+	/* Sniffer core structure */
+	struct snf_chan chan;
+	/* Pointer to device struct */
+	struct device *dev;
+	/* Registers */
+	u8 __iomem *regs;
+	/* Command string memory */
+	u32 __iomem *cmd_ram;
+	/* Bit number for the data IRQ */
+	int data_irq_bit;
+	/* Bit number for the FIFO full IRQ */
+	int full_irq_bit;
+	/* Channel enable register */
+	u8 __iomem *reg_enable;
+	/* Data FIFO register */
+	u8 __iomem *reg_fifo;
+	/* Max number of commands in the string */
+	int max_cmds;
+	/* Max matching bytes that can fit in a FIFO block */
+	int fifo_blk_words;
+	/* Number of bytes in the FIFO before the timestamp */
+	int nfb_before;
+	/* Number of bytes in the FIFO after the timestamp */
+	int nfb_after;
+	/* Channel active flag */
+	int started;
+	/* Last raw timestamp from the FIFO */
+	u32 raw_tstamp;
+	/* Channel name (only used by debug messages) */
+	char name[MAX_CHAN_NAME_SIZE];
+	/* Struct to hold data from a packet match */
+	struct snf_match_evt evt;
+	/* Cycle and time counters for tstamp handling */
+	struct cyclecounter cc;
+	struct timecounter tc;
+};
+
+int channel_init(
+		struct ether_snf_chan *ch,
+		struct platform_device *pdev,
+		void *regs,
+		int fifo_blk_words,
+		u32 tstamp_hz,
+		u32 tstamp_shift,
+		u32 tstamp_bits,
+		int tx);
+int channel_register(struct ether_snf_chan *ch, const char *name);
+int channel_unregister(struct ether_snf_chan *ch);
+void channel_data_available(struct ether_snf_chan *ch);
+int channel_suspend(struct ether_snf_chan *ch);
+int channel_resume(struct ether_snf_chan *ch);
+
+#endif
diff --git a/drivers/net/pkt-sniffer/backends/ether/hw.h b/drivers/net/pkt-sniffer/backends/ether/hw.h
new file mode 100644
index 0000000..edb1093
--- /dev/null
+++ b/drivers/net/pkt-sniffer/backends/ether/hw.h
@@ -0,0 +1,46 @@
+/*
+ * Ethernet Mii packet sniffer driver
+ *  - register map
+ *
+ * Copyright (C) 2015 Linn Products Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Written by:
+ * Stathis Voukelatos <stathis.voukelatos-zgcZaY4qg+21Qrn1Bg8BZw@public.gmane.org>
+ */
+#ifndef _ETHER_SNIFFER_HW_H_
+#define _ETHER_SNIFFER_HW_H_
+
+#include <linux/bitops.h>
+
+/* Registers */
+#define INTERRUPT_ENABLE        0x00
+#define SET_INTERRUPT_ENABLE    0x04
+#define CLEAR_INTERRUPT_ENABLE  0x08
+#define INTERRUPT_STATUS        0x0c
+#define TX_FIFO_DAT             0x10
+#define RX_FIFO_DAT             0x14
+#define TX_FIFO_OCC             0x18
+#define RX_FIFO_OCC             0x1c
+#define TX_SNIFFER_ENABLE       0x20
+#define RX_SNIFFER_ENABLE       0x24
+
+/* IRQ register bits */
+#define TX_DATA_IRQ_BIT         BIT(0)
+#define RX_DATA_IRQ_BIT         BIT(1)
+#define TX_FULL_IRQ_BIT         BIT(2)
+#define RX_FULL_IRQ_BIT         BIT(3)
+
+/* Enable register bits */
+#define ENABLE_BIT              BIT(0)
+
+#endif
+
diff --git a/drivers/net/pkt-sniffer/backends/ether/platform.c b/drivers/net/pkt-sniffer/backends/ether/platform.c
new file mode 100644
index 0000000..06b26b5
--- /dev/null
+++ b/drivers/net/pkt-sniffer/backends/ether/platform.c
@@ -0,0 +1,301 @@
+/*
+ * Ethernet Mii packet sniffer driver
+ *
+ * Copyright (C) 2015 Linn Products Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Written by:
+ * Stathis Voukelatos <stathis.voukelatos-zgcZaY4qg+21Qrn1Bg8BZw@public.gmane.org>
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include "../../core/snf_core.h"
+#include "hw.h"
+#include "channel.h"
+
+/* Names for the TX and RX channel net interfaces */
+static const char tx_channel_name[] = "snfethtx%d";
+static const char rx_channel_name[] = "snfethrx%d";
+
+struct ether_snf {
+	u8 __iomem *regs;
+	int irq;
+	struct clk *sys_clk;
+	struct clk *ts_clk;
+	struct ether_snf_chan txc;
+	struct ether_snf_chan rxc;
+};
+
+/* Interrupt handler */
+static irqreturn_t esnf_interrupt(int irq, void *dev_id)
+{
+	struct platform_device *pdev = (struct platform_device *)dev_id;
+	struct ether_snf *esnf = (struct ether_snf *)platform_get_drvdata(pdev);
+	u32 irq_status;
+
+	if (unlikely(esnf->irq != irq))
+		return IRQ_NONE;
+
+	irq_status = ioread32(esnf->regs + INTERRUPT_STATUS) &
+				 ioread32(esnf->regs + INTERRUPT_ENABLE);
+
+	dev_dbg(&pdev->dev, "irq: 0x%08x\n", irq_status);
+
+	/* TX FIFO full */
+	if (unlikely(irq_status & TX_FULL_IRQ_BIT))
+		dev_notice(&pdev->dev, "TX FIFO full\n");
+
+	/* RX FIFO full */
+	if (unlikely(irq_status & RX_FULL_IRQ_BIT))
+		dev_notice(&pdev->dev, "RX FIFO full\n");
+
+	/* TX match data available */
+	if (irq_status & TX_DATA_IRQ_BIT) {
+		dev_dbg(&pdev->dev, "TX data\n");
+		channel_data_available(&esnf->txc);
+	}
+
+	/* RX match data available */
+	if (irq_status & RX_DATA_IRQ_BIT) {
+		dev_dbg(&pdev->dev, "RX data\n");
+		channel_data_available(&esnf->rxc);
+	}
+
+	/* Clear interrupts */
+	iowrite32(irq_status, esnf->regs + INTERRUPT_STATUS);
+
+	return IRQ_HANDLED;
+}
+
+/* Called when the packet sniffer device is bound with the driver */
+static int esnf_driver_probe(struct platform_device *pdev)
+{
+	struct ether_snf *esnf;
+	struct resource *res;
+	int ret, irq;
+	u32 fifo_blk_words, ts_hz, ts_shift, ts_bits;
+	void __iomem *regs;
+	struct device_node *ofn = pdev->dev.of_node;
+
+	/* Allocate the device data structure */
+	esnf = devm_kzalloc(&pdev->dev, sizeof(*esnf), GFP_KERNEL);
+	if (!esnf)
+		return -ENOMEM;
+
+	/* Retrieve and remap register memory space */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
+	regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+	esnf->regs = regs;
+
+	if (!ofn)
+		return -ENODEV;
+
+	/* Read requirede properties from the device tree node */
+	ret = of_property_read_u32(
+				ofn,
+				"fifo-block-words",
+				&fifo_blk_words);
+	if (ret < 0)
+		return ret;
+
+	if (((fifo_blk_words - 1)*4) > MAX_MATCH_BYTES) {
+		dev_err(&pdev->dev,
+			"Invalid FIFO block size entry in device tree\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(
+				ofn,
+				"tstamp-hz",
+				&ts_hz);
+	if (ret < 0)
+		return ret;
+
+	ret = of_property_read_u32(
+				ofn,
+				"tstamp-shift",
+				&ts_shift);
+	if (ret < 0)
+		return ret;
+
+	ret = of_property_read_u32(
+				ofn,
+				"tstamp-bits",
+				&ts_bits);
+	if (ret < 0)
+		return ret;
+
+	/* Enable peripheral bus and timstamp clocks */
+	esnf->sys_clk = devm_clk_get(&pdev->dev, "sys");
+	if (IS_ERR(esnf->sys_clk)) {
+		ret = PTR_ERR(esnf->sys_clk);
+		return ret;
+	}
+	ret = clk_prepare_enable(esnf->sys_clk);
+	if (ret < 0)
+		return ret;
+
+	esnf->ts_clk = devm_clk_get(&pdev->dev, "tstamp");
+	if (IS_ERR(esnf->ts_clk)) {
+		ret = PTR_ERR(esnf->ts_clk);
+		goto fail1;
+	}
+	ret = clk_prepare_enable(esnf->ts_clk);
+	if (ret < 0)
+		goto fail1;
+
+	/* Initialise the TX and RX channels */
+	ret = channel_init(
+			&esnf->txc,
+			pdev,
+			regs,
+			fifo_blk_words,
+			ts_hz,
+			ts_shift,
+			ts_bits,
+			1);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to init TX channel (%d)\n", ret);
+		goto fail2;
+	}
+	ret = channel_init(
+			&esnf->rxc,
+			pdev,
+			regs,
+			fifo_blk_words,
+			ts_hz,
+			ts_shift,
+			ts_bits,
+			0);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to init RX channel (%d)\n", ret);
+		goto fail2;
+	}
+
+	/* Register the channels with the sniffer core module */
+	ret = channel_register(&esnf->txc, tx_channel_name);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to register TX chan (%d)\n", ret);
+		goto fail2;
+	}
+	ret = channel_register(&esnf->rxc, rx_channel_name);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to register RX chan (%d)\n", ret);
+		goto fail3;
+	}
+
+	platform_set_drvdata(pdev, esnf);
+
+	/* Register the interrupt handler */
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		goto fail4;
+	esnf->irq = irq;
+	ret = devm_request_irq(
+			&pdev->dev,
+			irq,
+			esnf_interrupt,
+			0,
+			KBUILD_MODNAME,
+			pdev);
+	if (ret < 0)
+		goto fail4;
+
+	return 0;
+
+fail4:
+	channel_unregister(&esnf->rxc);
+fail3:
+	channel_unregister(&esnf->txc);
+fail2:
+	clk_disable_unprepare(esnf->ts_clk);
+fail1:
+	clk_disable_unprepare(esnf->sys_clk);
+	return ret;
+}
+
+/* Called when the packet sniffer device unregisters with the driver */
+static int esnf_driver_remove(struct platform_device *pdev)
+{
+	struct ether_snf *esnf = (struct ether_snf *)platform_get_drvdata(pdev);
+	int ret;
+
+	ret = channel_unregister(&esnf->txc);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to unregister TX chan (%d)\n", ret);
+		return ret;
+	}
+	ret = channel_unregister(&esnf->rxc);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to unregister RX chan (%d)\n", ret);
+		return ret;
+	}
+	clk_disable_unprepare(esnf->ts_clk);
+	clk_disable_unprepare(esnf->sys_clk);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+/* Driver suspend method */
+static int esnf_driver_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct ether_snf *esnf = (struct ether_snf *)platform_get_drvdata(pdev);
+
+	channel_suspend(&esnf->txc);
+	channel_suspend(&esnf->rxc);
+	return 0;
+}
+
+/* Driver resume method */
+static int esnf_driver_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct ether_snf *esnf = (struct ether_snf *)platform_get_drvdata(pdev);
+
+	channel_resume(&esnf->txc);
+	channel_resume(&esnf->rxc);
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(esnf_pm_ops, esnf_driver_suspend, esnf_driver_resume);
+#endif
+
+static const struct of_device_id esnf_of_match_table[] = {
+	{ .compatible = "linn,eth-sniffer" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, esnf_of_match_table);
+
+static struct platform_driver esnf_platform_driver = {
+	.driver = {
+		.name           = KBUILD_MODNAME,
+#ifdef CONFIG_PM
+		.pm	        = &esnf_pm_ops,
+#endif
+		.of_match_table = esnf_of_match_table,
+	},
+	.probe = esnf_driver_probe,
+	.remove = esnf_driver_remove,
+};
+
+module_platform_driver(esnf_platform_driver);
+
+MODULE_DESCRIPTION("Linn Ethernet Packet Sniffer");
+MODULE_AUTHOR("Linn Products Ltd");
+MODULE_LICENSE("GPL v2");
+
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] net: Linn Ethernet Packet Sniffer driver

[Richard Cochran]

On Tue, Feb 17, 2015 at 02:03:30PM +0000, Stathis Voukelatos wrote:
> The command string for packet matching is stored in module RAM
> and consists of a sequence of 16-bit entries. Each entry includes
> an 8-bit command code and and 8-bit data value. Valid command
> codes are:
> 0 - Don't care
> 1 - Match: packet data must match command string byte
> 2 - Copy: packet data will be copied to FIFO
> 3 - Match/Stamp: if packet data matches string byte, a timestamp
>                  is copied into the FIFO
> 4 - Copy/Done: packet data will be copied into the FIFO.
>                This command terminates the command string.

Why do you need to expose this interface to user space at all?  Why
not just time stamp every frame?

How does the "Match" command work?  The frame must have one particular
byte?  That can't be right.  Please explain.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] net: Linn Ethernet Packet Sniffer driver

[Stathis Voukelatos]

Hi Richard,

On 18/02/15 21:08, Richard Cochran wrote:
> On Tue, Feb 17, 2015 at 02:03:30PM +0000, Stathis Voukelatos wrote:
>> The command string for packet matching is stored in module RAM
>> and consists of a sequence of 16-bit entries. Each entry includes
>> an 8-bit command code and and 8-bit data value. Valid command
>> codes are:
>> 0 - Don't care
>> 1 - Match: packet data must match command string byte
>> 2 - Copy: packet data will be copied to FIFO
>> 3 - Match/Stamp: if packet data matches string byte, a timestamp
>>                   is copied into the FIFO
>> 4 - Copy/Done: packet data will be copied into the FIFO.
>>                 This command terminates the command string.
>
> Why do you need to expose this interface to user space at all?  Why
> not just time stamp every frame?

To put this into context with an example, the use case this H/W module 
was originally developed for was to allow multiple audio receivers to 
synchronize with a single transmitter, eg. multi-room synchronised audio.
The interface needs to be public so that a user-space application can 
program a command string that will match packets that belong to the 
audio stream of interest, for this example.
In addition returning just a timestamp would not be enough in many 
cases. In the audio streaming use case mentioned above some additional
bytes from the packet payload need to be returned (with Copy commands) 
in order to associate the timestamp with a certain point in the audio 
stream.

>
> How does the "Match" command work?  The frame must have one particular
> byte?  That can't be right.  Please explain.

Actually, that is how the H/W works. Each Match command is followed by
a data value which must match the packet data byte at the corresponding
location. If there is no match processing of the packet stops.

>
> Thanks,
> Richard
>

Thank you,
Stathis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] net: Linn Ethernet Packet Sniffer driver

[Richard Cochran]

On Mon, Feb 23, 2015 at 09:37:24AM +0000, Stathis Voukelatos wrote:
> Hi Richard,
> 
> On 18/02/15 21:08, Richard Cochran wrote:
> >On Tue, Feb 17, 2015 at 02:03:30PM +0000, Stathis Voukelatos wrote:
> >>The command string for packet matching is stored in module RAM
> >>and consists of a sequence of 16-bit entries. Each entry includes
> >>an 8-bit command code and and 8-bit data value. Valid command
> >>codes are:
> >>0 - Don't care
> >>1 - Match: packet data must match command string byte
> >>2 - Copy: packet data will be copied to FIFO
> >>3 - Match/Stamp: if packet data matches string byte, a timestamp
> >>                  is copied into the FIFO
> >>4 - Copy/Done: packet data will be copied into the FIFO.
> >>                This command terminates the command string.
> >
> >Why do you need to expose this interface to user space at all?  Why
> >not just time stamp every frame?
> 
> To put this into context with an example, the use case this H/W
> module was originally developed for was to allow multiple audio
> receivers to synchronize with a single transmitter, eg. multi-room
> synchronised audio.

The Linux kernel already fully supports this kind of application via
the SIOCSHWTSTAMP, SO_TIMESTAMPING, and PHC mechanisms.  We certainly
don't need another another interface just for someone's warped
hardware design.

I suggest that you find a way to make your HW work within the existing
frame work.

> The interface needs to be public so that a user-space application
> can program a command string that will match packets that belong to
> the audio stream of interest, for this example.

Let the user program simply use a BPF for that.

> In addition returning just a timestamp would not be enough in many
> cases. In the audio streaming use case mentioned above some
> additional
> bytes from the packet payload need to be returned (with Copy
> commands) in order to associate the timestamp with a certain point
> in the audio stream.

The time stamp is *already* associated with a particular frame.

Just tell your driver to program the hardware to:

1. time stamp every frame
2. deliver every frame

Thats all you need.

> >How does the "Match" command work?  The frame must have one particular
> >byte?  That can't be right.  Please explain.
> 
> Actually, that is how the H/W works. Each Match command is followed by
> a data value which must match the packet data byte at the corresponding
> location. If there is no match processing of the packet stops.

And just what is the "corresponding location"?

Thanks,
Richard

Re: [PATCH v2 0/3] net: Linn Ethernet Packet Sniffer driver

[Stathis Voukelatos]

Hi Richard,

On 25/02/15 09:50, Richard Cochran wrote:
>
> The Linux kernel already fully supports this kind of application via
> the SIOCSHWTSTAMP, SO_TIMESTAMPING, and PHC mechanisms.  We certainly
> don't need another another interface just for someone's warped
> hardware design.
>
> I suggest that you find a way to make your HW work within the existing
> frame work.

The driver already uses that framework for returning timestamps to user 
space. It does not introduce any new interface.

>
>> The interface needs to be public so that a user-space application
>> can program a command string that will match packets that belong to
>> the audio stream of interest, for this example.
>
> Let the user program simply use a BPF for that.
>
>> In addition returning just a timestamp would not be enough in many
>> cases. In the audio streaming use case mentioned above some
>> additional
>> bytes from the packet payload need to be returned (with Copy
>> commands) in order to associate the timestamp with a certain point
>> in the audio stream.
>
> The time stamp is *already* associated with a particular frame.
>
> Just tell your driver to program the hardware to:
>
> 1. time stamp every frame
> 2. deliver every frame
>
> Thats all you need.
>

The H/W could not support that:
We have a "Match/Stamp" command, so the packet byte needs to match the 
value following the command, in order for a timestamp to be generated, 
otherwise the packet is dropped.
In addition, due to FIFO size limitations up to 128 bytes (including the 
timestamp) can be returned (through Copy commands) from each packet.

The module was designed to be able to configure it to sniff packets 
belonging to a certain application level stream and from each matching 
packet return a timestamp and some bytes (from eg. the application layer 
protocol header) that would be useful to the application.

BPF could accomplish that too, but timestamps will not be as accurate
without H/W support.

I understand that the device needs to be configured with a proprietary
command stream, but all interfacing with user-space is done using 
existing frameworks (AF_PACKET, SIOCSHWTSTAMP, cmsg)

>> Actually, that is how the H/W works. Each Match command is followed by
>> a data value which must match the packet data byte at the corresponding
>> location. If there is no match processing of the packet stops.
>
> And just what is the "corresponding location"?

The command string is made up of a sequence of <CMD><DATA> pairs. Take 
this for example: {0x00, 0x00, 0x01, 0x55, 0x02, 0x00, 0x04, 0x00}.
First command is Don't Care (0x00), ie 1st byte of Ethernet frame is 
just skipped.
Second command is Match (0x01), ie if the 2nd byte of the Ethernet frame 
is 0x55 processing continues otherwise packet is dropped.
Third command is Copy (0x02), ie 3rd byte of the packet is copied to the 
H/W FIFO to be returned to the user
Fourth command is Copy/Done (0x04), ie 4th packet byte is also copied to 
the FIFO and processing stops.
Then an IRQ is generated, data (2 bytes) are read from the FIFO and
delivered through an AF_PACKET socket.

In v4 of the patch I tried to improve the documentation of some of these 
points.
>
> Thanks,
> Richard
>

Thanks,
Stathis

Re: [PATCH v2 0/3] net: Linn Ethernet Packet Sniffer driver

[Richard Cochran]

On Wed, Feb 25, 2015 at 10:53:11AM +0000, Stathis Voukelatos wrote:
> On 25/02/15 09:50, Richard Cochran wrote:
> >
> >The Linux kernel already fully supports this kind of application via
> >the SIOCSHWTSTAMP, SO_TIMESTAMPING, and PHC mechanisms.  We certainly
> >don't need another another interface just for someone's warped
> >hardware design.
> >
> >I suggest that you find a way to make your HW work within the existing
> >frame work.
> 
> The driver already uses that framework for returning timestamps to
> user space. It does not introduce any new interface.

No, it only abuses the interfaces that we already have.  The time
stamp handling is not "within the existing frame work" by any means.
However, there is a way to integrate this hardware properly...

> The H/W could not support that:

With these new explanations, now I understand the code and how this is
supposed to work.

I'll put further comments in a reply to your newer series.

Thanks,
Richard