Re: [PATCH 3/3] gianfar: fix endianness for hardware timestamp
Hi, Yangbo Lu wrote: >> -Original Message- >> From: Richard Cochran [mailto:richardcoch...@gmail.com] >> Sent: Monday, February 22, 2016 4:48 PM >> To: Yangbo Lu >> Cc: netdev@vger.kernel.org; Claudiu Manoil >> Subject: Re: [PATCH 3/3] gianfar: fix endianness for hardware timestamp >> >> On Mon, Feb 22, 2016 at 02:49:33PM +0800, Yangbo Lu wrote: >>> @@ -2708,6 +2708,7 @@ static void gfar_clean_tx_ring(struct >> gfar_priv_tx_q *tx_queue) >>> struct skb_shared_hwtstamps shhwtstamps; >>> u64 *ns = (u64 *)(((uintptr_t)skb->data + 0x10) & >>> ~0x7UL); >>> + *ns = be64_to_cpu(*ns); >>> >>> memset(, 0, sizeof(shhwtstamps)); >>> shhwtstamps.hwtstamp = ns_to_ktime(*ns); >> >> There is no point in modifying the buffer data in place. >> >> Instead, do this: >> >> memset(, 0, sizeof(shhwtstamps)); >> shhwtstamps.hwtstamp = ns_to_ktime(be64_to_cpu(*ns)); >> >> or this: >> >> u64 ns, *ptr = (u64 *)(((uintptr_t)skb->data + 0x10) & >> ~0x7UL); >> ns = be64_to_cpu(*ptr); >> >> memset(, 0, sizeof(shhwtstamps)); >> shhwtstamps.hwtstamp = ns_to_ktime(ns); >> > > [Lu Yangbo-B47093] I will modify codes according your suggestion. > Thank you so much! You may want to use PTR_ALIGN() and be64_to_cpup() here. --yoshfuji > >>> @@ -3037,6 +3038,7 @@ static void gfar_process_frame(struct net_device >> *ndev, struct sk_buff *skb) >>> if (priv->hwts_rx_en) { >>> struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); >>> u64 *ns = (u64 *) skb->data; >>> + *ns = be64_to_cpu(*ns); >>> >>> memset(shhwtstamps, 0, sizeof(*shhwtstamps)); >>> shhwtstamps->hwtstamp = ns_to_ktime(*ns); >> >> Same here. >> >> Thanks, >> Richard -- Hideaki Yoshifuji <hideaki.yoshif...@miraclelinux.com> Technical Division, MIRACLE LINUX CORPORATION
RE: [PATCH 3/3] gianfar: fix endianness for hardware timestamp
> -Original Message- > From: Richard Cochran [mailto:richardcoch...@gmail.com] > Sent: Monday, February 22, 2016 4:48 PM > To: Yangbo Lu > Cc: netdev@vger.kernel.org; Claudiu Manoil > Subject: Re: [PATCH 3/3] gianfar: fix endianness for hardware timestamp > > On Mon, Feb 22, 2016 at 02:49:33PM +0800, Yangbo Lu wrote: > > @@ -2708,6 +2708,7 @@ static void gfar_clean_tx_ring(struct > gfar_priv_tx_q *tx_queue) > > struct skb_shared_hwtstamps shhwtstamps; > > u64 *ns = (u64 *)(((uintptr_t)skb->data + 0x10) & > > ~0x7UL); > > + *ns = be64_to_cpu(*ns); > > > > memset(, 0, sizeof(shhwtstamps)); > > shhwtstamps.hwtstamp = ns_to_ktime(*ns); > > There is no point in modifying the buffer data in place. > > Instead, do this: > > memset(, 0, sizeof(shhwtstamps)); > shhwtstamps.hwtstamp = ns_to_ktime(be64_to_cpu(*ns)); > > or this: > > u64 ns, *ptr = (u64 *)(((uintptr_t)skb->data + 0x10) & > ~0x7UL); > ns = be64_to_cpu(*ptr); > > memset(, 0, sizeof(shhwtstamps)); > shhwtstamps.hwtstamp = ns_to_ktime(ns); > [Lu Yangbo-B47093] I will modify codes according your suggestion. Thank you so much! > > @@ -3037,6 +3038,7 @@ static void gfar_process_frame(struct net_device > *ndev, struct sk_buff *skb) > > if (priv->hwts_rx_en) { > > struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); > > u64 *ns = (u64 *) skb->data; > > + *ns = be64_to_cpu(*ns); > > > > memset(shhwtstamps, 0, sizeof(*shhwtstamps)); > > shhwtstamps->hwtstamp = ns_to_ktime(*ns); > > Same here. > > Thanks, > Richard
Re: [PATCH 3/3] gianfar: fix endianness for hardware timestamp
On Mon, Feb 22, 2016 at 02:49:33PM +0800, Yangbo Lu wrote: > Signed-off-by: Yangbo LuCommit message is missing. Thanks, Richard
Re: [PATCH 3/3] gianfar: fix endianness for hardware timestamp
On Mon, Feb 22, 2016 at 02:49:33PM +0800, Yangbo Lu wrote: > @@ -2708,6 +2708,7 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q > *tx_queue) > struct skb_shared_hwtstamps shhwtstamps; > u64 *ns = (u64 *)(((uintptr_t)skb->data + 0x10) & > ~0x7UL); > + *ns = be64_to_cpu(*ns); > > memset(, 0, sizeof(shhwtstamps)); > shhwtstamps.hwtstamp = ns_to_ktime(*ns); There is no point in modifying the buffer data in place. Instead, do this: memset(, 0, sizeof(shhwtstamps)); shhwtstamps.hwtstamp = ns_to_ktime(be64_to_cpu(*ns)); or this: u64 ns, *ptr = (u64 *)(((uintptr_t)skb->data + 0x10) & ~0x7UL); ns = be64_to_cpu(*ptr); memset(, 0, sizeof(shhwtstamps)); shhwtstamps.hwtstamp = ns_to_ktime(ns); > @@ -3037,6 +3038,7 @@ static void gfar_process_frame(struct net_device *ndev, > struct sk_buff *skb) > if (priv->hwts_rx_en) { > struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); > u64 *ns = (u64 *) skb->data; > + *ns = be64_to_cpu(*ns); > > memset(shhwtstamps, 0, sizeof(*shhwtstamps)); > shhwtstamps->hwtstamp = ns_to_ktime(*ns); Same here. Thanks, Richard