* [PATCH 2/2] ksz884x: fix Endian
@ 2012-07-05 2:06 roy.qing.li
2012-07-06 22:40 ` Ben Hutchings
0 siblings, 1 reply; 5+ messages in thread
From: roy.qing.li @ 2012-07-05 2:06 UTC (permalink / raw)
To: netdev; +Cc: Tristram.Ha
From: Li RongQing <roy.qing.li@gmail.com>
ETH_P_IP is host Endian, skb->protocol is big Endian, when
compare them, we should change skb->protocol from big endian
to host endian, ntohs, not htons.
CC: Tristram Ha <Tristram.Ha@micrel.com>
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
drivers/net/ethernet/micrel/ksz884x.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/micrel/ksz884x.c b/drivers/net/ethernet/micrel/ksz884x.c
index eaf9ff0..d9727f7 100644
--- a/drivers/net/ethernet/micrel/ksz884x.c
+++ b/drivers/net/ethernet/micrel/ksz884x.c
@@ -4882,7 +4882,7 @@ static netdev_tx_t netdev_tx(struct sk_buff *skb, struct net_device *dev)
if (left) {
if (left < num ||
((CHECKSUM_PARTIAL == skb->ip_summed) &&
- (ETH_P_IPV6 == htons(skb->protocol)))) {
+ (ETH_P_IPV6 == ntohs(skb->protocol)))) {
struct sk_buff *org_skb = skb;
skb = netdev_alloc_skb(dev, org_skb->len);
--
1.7.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ksz884x: fix Endian
2012-07-05 2:06 [PATCH 2/2] ksz884x: fix Endian roy.qing.li
@ 2012-07-06 22:40 ` Ben Hutchings
2012-07-09 5:26 ` RongQing Li
0 siblings, 1 reply; 5+ messages in thread
From: Ben Hutchings @ 2012-07-06 22:40 UTC (permalink / raw)
To: roy.qing.li; +Cc: netdev, Tristram.Ha
On Thu, 2012-07-05 at 10:06 +0800, roy.qing.li@gmail.com wrote:
> From: Li RongQing <roy.qing.li@gmail.com>
>
> ETH_P_IP is host Endian, skb->protocol is big Endian, when
> compare them, we should change skb->protocol from big endian
> to host endian, ntohs, not htons.
>
> CC: Tristram Ha <Tristram.Ha@micrel.com>
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
> ---
> drivers/net/ethernet/micrel/ksz884x.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ethernet/micrel/ksz884x.c b/drivers/net/ethernet/micrel/ksz884x.c
> index eaf9ff0..d9727f7 100644
> --- a/drivers/net/ethernet/micrel/ksz884x.c
> +++ b/drivers/net/ethernet/micrel/ksz884x.c
> @@ -4882,7 +4882,7 @@ static netdev_tx_t netdev_tx(struct sk_buff *skb, struct net_device *dev)
> if (left) {
> if (left < num ||
> ((CHECKSUM_PARTIAL == skb->ip_summed) &&
> - (ETH_P_IPV6 == htons(skb->protocol)))) {
> + (ETH_P_IPV6 == ntohs(skb->protocol)))) {
This should really be changed to the idiomatic 'skb->protocol ==
htons(ETH_P_IPV6)'. For the current code, the compiler will probably
generate a run-time byte-swap for little-endian systems.
Ben.
> struct sk_buff *org_skb = skb;
>
> skb = netdev_alloc_skb(dev, org_skb->len);
--
Ben Hutchings, Staff 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ksz884x: fix Endian
2012-07-06 22:40 ` Ben Hutchings
@ 2012-07-09 5:26 ` RongQing Li
2012-07-09 5:30 ` RongQing Li
2012-07-09 5:44 ` Joe Perches
0 siblings, 2 replies; 5+ messages in thread
From: RongQing Li @ 2012-07-09 5:26 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, Tristram.Ha
2012/7/7, Ben Hutchings <bhutchings@solarflare.com>:
> On Thu, 2012-07-05 at 10:06 +0800, roy.qing.li@gmail.com wrote:
>> From: Li RongQing <roy.qing.li@gmail.com>
>>
>> ETH_P_IP is host Endian, skb->protocol is big Endian, when
>> compare them, we should change skb->protocol from big endian
>> to host endian, ntohs, not htons.
>>
>> CC: Tristram Ha <Tristram.Ha@micrel.com>
>> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
>> ---
>> drivers/net/ethernet/micrel/ksz884x.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/micrel/ksz884x.c
>> b/drivers/net/ethernet/micrel/ksz884x.c
>> index eaf9ff0..d9727f7 100644
>> --- a/drivers/net/ethernet/micrel/ksz884x.c
>> +++ b/drivers/net/ethernet/micrel/ksz884x.c
>> @@ -4882,7 +4882,7 @@ static netdev_tx_t netdev_tx(struct sk_buff *skb,
>> struct net_device *dev)
>> if (left) {
>> if (left < num ||
>> ((CHECKSUM_PARTIAL == skb->ip_summed) &&
>> - (ETH_P_IPV6 == htons(skb->protocol)))) {
>> + (ETH_P_IPV6 == ntohs(skb->protocol)))) {
>
> This should really be changed to the idiomatic 'skb->protocol ==
> htons(ETH_P_IPV6)'. For the current code, the compiler will probably
> generate a run-time byte-swap for little-endian systems.
>
> Ben.
>
>> struct sk_buff *org_skb = skb;
>>
>> skb = netdev_alloc_skb(dev, org_skb->len);
>
> --
> Ben Hutchings, Staff 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.
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ksz884x: fix Endian
2012-07-09 5:26 ` RongQing Li
@ 2012-07-09 5:30 ` RongQing Li
2012-07-09 5:44 ` Joe Perches
1 sibling, 0 replies; 5+ messages in thread
From: RongQing Li @ 2012-07-09 5:30 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, Tristram.Ha
Please ignore the first reply.
OK, I will change it as Ben's suggestion.
Thanks
-Roy
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ksz884x: fix Endian
2012-07-09 5:26 ` RongQing Li
2012-07-09 5:30 ` RongQing Li
@ 2012-07-09 5:44 ` Joe Perches
1 sibling, 0 replies; 5+ messages in thread
From: Joe Perches @ 2012-07-09 5:44 UTC (permalink / raw)
To: RongQing Li; +Cc: Ben Hutchings, netdev, Tristram.Ha
On Mon, 2012-07-09 at 13:26 +0800, RongQing Li wrote:
> 2012/7/7, Ben Hutchings <bhutchings@solarflare.com>:
> > On Thu, 2012-07-05 at 10:06 +0800, roy.qing.li@gmail.com wrote:
> >> ETH_P_IP is host Endian, skb->protocol is big Endian, when
> >> compare them, we should change skb->protocol from big endian
> >> to host endian, ntohs, not htons.
[]
> >> diff --git a/drivers/net/ethernet/micrel/ksz884x.c
[]
> >> @@ -4882,7 +4882,7 @@ static netdev_tx_t netdev_tx(struct sk_buff *skb,
> >> struct net_device *dev)
> >> if (left) {
> >> if (left < num ||
> >> ((CHECKSUM_PARTIAL == skb->ip_summed) &&
> >> - (ETH_P_IPV6 == htons(skb->protocol)))) {
> >> + (ETH_P_IPV6 == ntohs(skb->protocol)))) {
> >
> > This should really be changed to the idiomatic 'skb->protocol ==
> > htons(ETH_P_IPV6)'. For the current code, the compiler will probably
> > generate a run-time byte-swap for little-endian systems.
True. Perhaps this would be better written as:
if (left) {
if (left < num ||
(ip->ip_summed == CHECKSUM_PARTIAL &&
skb->protocol == htons(ETH_P_IPV6))) {
etc...
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-07-09 5:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-05 2:06 [PATCH 2/2] ksz884x: fix Endian roy.qing.li
2012-07-06 22:40 ` Ben Hutchings
2012-07-09 5:26 ` RongQing Li
2012-07-09 5:30 ` RongQing Li
2012-07-09 5:44 ` Joe Perches
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.