All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Hutchings <bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
To: Richard Cochran <richardcochran-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	Alan Cox <alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>,
	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>,
	David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>,
	John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Krzysztof Halasa <khc-9GfyWEdoJtJmR6Xm/wNWPw@public.gmane.org>,
	Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
	Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>,
	Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>,
	Benjamin Herrenschmidt
	<benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>,
	Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>,
	Paul Mackerras <paulus-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>,
	Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Subject: Re: [PATCH V14 3/4] ptp: Added a clock driver for the IXP46x.
Date: Mon, 18 Apr 2011 21:53:06 +0100	[thread overview]
Message-ID: <1303159986.2857.56.camel@bwh-desktop> (raw)
In-Reply-To: <dbee2487a57fd7f41e4efbbd2ddfb1313b8d71bc.1303107532.git.richard.cochran-3mrvs1K0uXizZXS1Dc/lvw@public.gmane.org>

On Mon, 2011-04-18 at 08:29 +0200, Richard Cochran wrote:
> This patch adds a driver for the hardware time stamping unit found on the
> IXP465. The basic clock operations and an external trigger are implemented.
[...]
> --- a/drivers/net/arm/ixp4xx_eth.c
> +++ b/drivers/net/arm/ixp4xx_eth.c
[...]
> @@ -246,6 +255,169 @@ static int ports_open;
>  static struct port *npe_port_tab[MAX_NPES];
>  static struct dma_pool *dma_pool;
>  
> +static struct sock_filter ptp_filter[] = {
> +	PTP_FILTER
> +};
> +
> +static int ixp_ptp_match(struct sk_buff *skb, u16 uid_hi, u32 uid_lo, u16 seq)
> +{
> +	unsigned int type;
> +	u16 *hi, *id;
> +	u8 *lo, *data = skb->data;
> +
> +	type = sk_run_filter(skb, ptp_filter);
> +
> +	if (PTP_CLASS_V1_IPV4 == type) {
> +
> +		id = (u16 *)(data + 42 + 30);
> +		hi = (u16 *)(data + 42 + 22);
> +		lo = data + 42 + 24;
[...]

PTP_FILTER does not verify that the packet length is sufficient to hold
a complete PTP header, nor does it require that the IPv4 header length
is 5 (i.e. 20 bytes).  So you have to check those here rather than using
magic numbers.

I think you also need to use be16_to_cpup() to read 'id' and 'hi', since
the host byte order may vary.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

WARNING: multiple messages have this Message-ID (diff)
From: Ben Hutchings <bhutchings@solarflare.com>
To: Richard Cochran <richardcochran@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Rodolfo Giometti <giometti@linux.it>,
	Arnd Bergmann <arnd@arndb.de>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-api@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-kernel@vger.kernel.org,
	Russell King <linux@arm.linux.org.uk>,
	Paul Mackerras <paulus@samba.org>,
	John Stultz <john.stultz@linaro.org>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	netdev@vger.kernel.org, Mike Frysinger <vapier@gentoo.org>,
	Christoph Lameter <cl@linux.com>,
	linuxppc-dev@lists.ozlabs.org, David Miller <davem@davemloft.net>,
	linux-arm-kernel@lists.infradead.org,
	Krzysztof Halasa <khc@pm.waw.pl>
Subject: Re: [PATCH V14 3/4] ptp: Added a clock driver for the IXP46x.
Date: Mon, 18 Apr 2011 21:53:06 +0100	[thread overview]
Message-ID: <1303159986.2857.56.camel@bwh-desktop> (raw)
In-Reply-To: <dbee2487a57fd7f41e4efbbd2ddfb1313b8d71bc.1303107532.git.richard.cochran@omicron.at>

On Mon, 2011-04-18 at 08:29 +0200, Richard Cochran wrote:
> This patch adds a driver for the hardware time stamping unit found on the
> IXP465. The basic clock operations and an external trigger are implemented.
[...]
> --- a/drivers/net/arm/ixp4xx_eth.c
> +++ b/drivers/net/arm/ixp4xx_eth.c
[...]
> @@ -246,6 +255,169 @@ static int ports_open;
>  static struct port *npe_port_tab[MAX_NPES];
>  static struct dma_pool *dma_pool;
>  
> +static struct sock_filter ptp_filter[] = {
> +	PTP_FILTER
> +};
> +
> +static int ixp_ptp_match(struct sk_buff *skb, u16 uid_hi, u32 uid_lo, u16 seq)
> +{
> +	unsigned int type;
> +	u16 *hi, *id;
> +	u8 *lo, *data = skb->data;
> +
> +	type = sk_run_filter(skb, ptp_filter);
> +
> +	if (PTP_CLASS_V1_IPV4 == type) {
> +
> +		id = (u16 *)(data + 42 + 30);
> +		hi = (u16 *)(data + 42 + 22);
> +		lo = data + 42 + 24;
[...]

PTP_FILTER does not verify that the packet length is sufficient to hold
a complete PTP header, nor does it require that the IPv4 header length
is 5 (i.e. 20 bytes).  So you have to check those here rather than using
magic numbers.

I think you also need to use be16_to_cpup() to read 'id' and 'hi', since
the host byte order may vary.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

WARNING: multiple messages have this Message-ID (diff)
From: bhutchings@solarflare.com (Ben Hutchings)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V14 3/4] ptp: Added a clock driver for the IXP46x.
Date: Mon, 18 Apr 2011 21:53:06 +0100	[thread overview]
Message-ID: <1303159986.2857.56.camel@bwh-desktop> (raw)
In-Reply-To: <dbee2487a57fd7f41e4efbbd2ddfb1313b8d71bc.1303107532.git.richard.cochran@omicron.at>

On Mon, 2011-04-18 at 08:29 +0200, Richard Cochran wrote:
> This patch adds a driver for the hardware time stamping unit found on the
> IXP465. The basic clock operations and an external trigger are implemented.
[...]
> --- a/drivers/net/arm/ixp4xx_eth.c
> +++ b/drivers/net/arm/ixp4xx_eth.c
[...]
> @@ -246,6 +255,169 @@ static int ports_open;
>  static struct port *npe_port_tab[MAX_NPES];
>  static struct dma_pool *dma_pool;
>  
> +static struct sock_filter ptp_filter[] = {
> +	PTP_FILTER
> +};
> +
> +static int ixp_ptp_match(struct sk_buff *skb, u16 uid_hi, u32 uid_lo, u16 seq)
> +{
> +	unsigned int type;
> +	u16 *hi, *id;
> +	u8 *lo, *data = skb->data;
> +
> +	type = sk_run_filter(skb, ptp_filter);
> +
> +	if (PTP_CLASS_V1_IPV4 == type) {
> +
> +		id = (u16 *)(data + 42 + 30);
> +		hi = (u16 *)(data + 42 + 22);
> +		lo = data + 42 + 24;
[...]

PTP_FILTER does not verify that the packet length is sufficient to hold
a complete PTP header, nor does it require that the IPv4 header length
is 5 (i.e. 20 bytes).  So you have to check those here rather than using
magic numbers.

I think you also need to use be16_to_cpup() to read 'id' and 'hi', since
the host byte order may vary.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

WARNING: multiple messages have this Message-ID (diff)
From: Ben Hutchings <bhutchings@solarflare.com>
To: Richard Cochran <richardcochran@gmail.com>
Cc: linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
	netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Arnd Bergmann <arnd@arndb.de>, Christoph Lameter <cl@linux.com>,
	David Miller <davem@davemloft.net>,
	John Stultz <john.stultz@linaro.org>,
	Krzysztof Halasa <khc@pm.waw.pl>,
	Peter Zijlstra <peterz@infradead.org>,
	Rodolfo Giometti <giometti@linux.it>,
	Thomas Gleixner <tglx@linutronix.de>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Mike Frysinger <vapier@gentoo.org>,
	Paul Mackerras <paulus@samba.org>,
	Russell King <linux@arm.linux.org.uk>
Subject: Re: [PATCH V14 3/4] ptp: Added a clock driver for the IXP46x.
Date: Mon, 18 Apr 2011 21:53:06 +0100	[thread overview]
Message-ID: <1303159986.2857.56.camel@bwh-desktop> (raw)
In-Reply-To: <dbee2487a57fd7f41e4efbbd2ddfb1313b8d71bc.1303107532.git.richard.cochran@omicron.at>

On Mon, 2011-04-18 at 08:29 +0200, Richard Cochran wrote:
> This patch adds a driver for the hardware time stamping unit found on the
> IXP465. The basic clock operations and an external trigger are implemented.
[...]
> --- a/drivers/net/arm/ixp4xx_eth.c
> +++ b/drivers/net/arm/ixp4xx_eth.c
[...]
> @@ -246,6 +255,169 @@ static int ports_open;
>  static struct port *npe_port_tab[MAX_NPES];
>  static struct dma_pool *dma_pool;
>  
> +static struct sock_filter ptp_filter[] = {
> +	PTP_FILTER
> +};
> +
> +static int ixp_ptp_match(struct sk_buff *skb, u16 uid_hi, u32 uid_lo, u16 seq)
> +{
> +	unsigned int type;
> +	u16 *hi, *id;
> +	u8 *lo, *data = skb->data;
> +
> +	type = sk_run_filter(skb, ptp_filter);
> +
> +	if (PTP_CLASS_V1_IPV4 == type) {
> +
> +		id = (u16 *)(data + 42 + 30);
> +		hi = (u16 *)(data + 42 + 22);
> +		lo = data + 42 + 24;
[...]

PTP_FILTER does not verify that the packet length is sufficient to hold
a complete PTP header, nor does it require that the IPv4 header length
is 5 (i.e. 20 bytes).  So you have to check those here rather than using
magic numbers.

I think you also need to use be16_to_cpup() to read 'id' and 'hi', since
the host byte order may vary.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


  parent reply	other threads:[~2011-04-18 20:53 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-18  6:27 [PATCH V14 0/4] ptp: IEEE 1588 hardware clock support Richard Cochran
2011-04-18  6:27 ` Richard Cochran
2011-04-18  6:27 ` Richard Cochran
2011-04-18  6:27 ` Richard Cochran
2011-04-18  6:29 ` [PATCH V14 2/4] ptp: Added a clock that uses the eTSEC found on the MPC85xx Richard Cochran
2011-04-18  6:29   ` Richard Cochran
2011-04-18  6:29   ` Richard Cochran
     [not found] ` <cover.1303107532.git.richard.cochran-3mrvs1K0uXizZXS1Dc/lvw@public.gmane.org>
2011-04-18  6:28   ` [PATCH V14 1/4] ptp: Added a brand new class driver for ptp clocks Richard Cochran
2011-04-18  6:28     ` Richard Cochran
2011-04-18  6:28     ` Richard Cochran
2011-04-18  6:28     ` Richard Cochran
     [not found]     ` <7cabcc0bbd14733fa979fe08deccab8939204f24.1303107532.git.richard.cochran-3mrvs1K0uXizZXS1Dc/lvw@public.gmane.org>
2011-04-18  6:54       ` Arnd Bergmann
2011-04-18  6:54         ` Arnd Bergmann
2011-04-18  6:54         ` Arnd Bergmann
2011-04-18  6:54         ` Arnd Bergmann
2011-04-18  6:29   ` [PATCH V14 3/4] ptp: Added a clock driver for the IXP46x Richard Cochran
2011-04-18  6:29     ` Richard Cochran
2011-04-18  6:29     ` Richard Cochran
2011-04-18  6:29     ` Richard Cochran
     [not found]     ` <dbee2487a57fd7f41e4efbbd2ddfb1313b8d71bc.1303107532.git.richard.cochran-3mrvs1K0uXizZXS1Dc/lvw@public.gmane.org>
2011-04-18  6:56       ` Arnd Bergmann
2011-04-18  6:56         ` Arnd Bergmann
2011-04-18  6:56         ` Arnd Bergmann
2011-04-18  6:56         ` Arnd Bergmann
     [not found]         ` <201104180856.04186.arnd-r2nGTMty4D4@public.gmane.org>
2011-04-18  8:06           ` Richard Cochran
2011-04-18  8:06             ` Richard Cochran
2011-04-18  8:06             ` Richard Cochran
2011-04-18  8:06             ` Richard Cochran
2011-04-18  8:16             ` Richard Cochran
2011-04-18  8:16               ` Richard Cochran
2011-04-18  8:16               ` Richard Cochran
2011-04-18  8:21             ` Arnd Bergmann
2011-04-18  8:21               ` Arnd Bergmann
2011-04-18  8:21               ` Arnd Bergmann
     [not found]               ` <201104181021.01279.arnd-r2nGTMty4D4@public.gmane.org>
2011-04-18  8:26                 ` Arnd Bergmann
2011-04-18  8:26                   ` Arnd Bergmann
2011-04-18  8:26                   ` Arnd Bergmann
2011-04-18  8:26                   ` Arnd Bergmann
2011-04-18 20:53       ` Ben Hutchings [this message]
2011-04-18 20:53         ` Ben Hutchings
2011-04-18 20:53         ` Ben Hutchings
2011-04-18 20:53         ` Ben Hutchings
2011-04-18  6:30   ` [PATCH V14 4/4] ptp: Added a clock driver for the National Semiconductor PHYTER Richard Cochran
2011-04-18  6:30     ` Richard Cochran
2011-04-18  6:30     ` Richard Cochran
2011-04-18  6:30     ` Richard Cochran
2011-04-18 20:57     ` Ben Hutchings
2011-04-18 20:57       ` Ben Hutchings
2011-04-18 20:57       ` Ben Hutchings
2011-04-22  9:21       ` Richard Cochran
2011-04-22  9:21         ` Richard Cochran
2011-04-22  9:21         ` Richard Cochran

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1303159986.2857.56.camel@bwh-desktop \
    --to=bhutchings-s/n/euqhgbpzrors9yw3xa@public.gmane.org \
    --cc=alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org \
    --cc=arnd-r2nGTMty4D4@public.gmane.org \
    --cc=benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org \
    --cc=cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org \
    --cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org \
    --cc=devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org \
    --cc=giometti-k2GhghHVRtY@public.gmane.org \
    --cc=john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=khc-9GfyWEdoJtJmR6Xm/wNWPw@public.gmane.org \
    --cc=linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org \
    --cc=linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org \
    --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=paulus-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org \
    --cc=peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \
    --cc=richardcochran-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org \
    --cc=vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.