From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Hutchings Subject: Re: [PATCH V14 3/4] ptp: Added a clock driver for the IXP46x. Date: Mon, 18 Apr 2011 21:53:06 +0100 Message-ID: <1303159986.2857.56.camel@bwh-desktop> References: Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-api-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Richard Cochran 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 , Arnd Bergmann , Christoph Lameter , David Miller , John Stultz , Krzysztof Halasa , Peter Zijlstra , Rodolfo Giometti , Thomas Gleixner , Benjamin Herrenschmidt , Mike Frysinger , Paul Mackerras , Russell King List-Id: linux-api@vger.kernel.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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from exchange.solarflare.com (exchange.solarflare.com [216.237.3.220]) by ozlabs.org (Postfix) with ESMTP id ACC71B6FCE for ; Tue, 19 Apr 2011 06:53:15 +1000 (EST) Subject: Re: [PATCH V14 3/4] ptp: Added a clock driver for the IXP46x. From: Ben Hutchings To: Richard Cochran In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Date: Mon, 18 Apr 2011 21:53:06 +0100 Message-ID: <1303159986.2857.56.camel@bwh-desktop> Mime-Version: 1.0 Cc: Thomas Gleixner , Rodolfo Giometti , Arnd Bergmann , Peter Zijlstra , linux-api@vger.kernel.org, devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org, Russell King , Paul Mackerras , John Stultz , Alan Cox , netdev@vger.kernel.org, Mike Frysinger , Christoph Lameter , linuxppc-dev@lists.ozlabs.org, David Miller , linux-arm-kernel@lists.infradead.org, Krzysztof Halasa List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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. From mboxrd@z Thu Jan 1 00:00:00 1970 From: bhutchings@solarflare.com (Ben Hutchings) Date: Mon, 18 Apr 2011 21:53:06 +0100 Subject: [PATCH V14 3/4] ptp: Added a clock driver for the IXP46x. In-Reply-To: References: Message-ID: <1303159986.2857.56.camel@bwh-desktop> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752397Ab1DRUxO (ORCPT ); Mon, 18 Apr 2011 16:53:14 -0400 Received: from mail.solarflare.com ([216.237.3.220]:19227 "EHLO exchange.solarflare.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751494Ab1DRUxM (ORCPT ); Mon, 18 Apr 2011 16:53:12 -0400 Subject: Re: [PATCH V14 3/4] ptp: Added a clock driver for the IXP46x. From: Ben Hutchings To: Richard Cochran 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 , Arnd Bergmann , Christoph Lameter , David Miller , John Stultz , Krzysztof Halasa , Peter Zijlstra , Rodolfo Giometti , Thomas Gleixner , Benjamin Herrenschmidt , Mike Frysinger , Paul Mackerras , Russell King In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Organization: Solarflare Communications Date: Mon, 18 Apr 2011 21:53:06 +0100 Message-ID: <1303159986.2857.56.camel@bwh-desktop> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 (2.32.1-1.fc14) Content-Transfer-Encoding: 7bit X-OriginalArrivalTime: 18 Apr 2011 20:53:11.0455 (UTC) FILETIME=[A10182F0:01CBFE0A] X-TM-AS-Product-Ver: SMEX-8.0.0.1181-6.500.1024-18080.005 X-TM-AS-Result: No--12.618400-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.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.