* [BK PATCH] Make eth.c independent of dev->hard_header_len
@ 2002-09-29 18:34 Kai Germaschewski
2002-09-29 18:45 ` Jeff Garzik
2002-09-30 4:36 ` David S. Miller
0 siblings, 2 replies; 6+ messages in thread
From: Kai Germaschewski @ 2002-09-29 18:34 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev
Dave,
I'd like to submit to small changes to net/ethernet/eth.c, which makes
it work independently of the value of dev->hard_header_len.
It does not change any existing behavior, since for eth_header(),
only the sign of the return value is changed anyway and eth_type_trans()
is only used for devices where dev->hard_header_len == ETH_HLEN anyway.
This patch makes (IMO) eth.c more robust by using ETH_HLEN instead.
eth_header() already does a skb_push(, ETH_HLEN) anyway, so it makes
more sense to return ETH_HLEN instead of ->hard_header_len.
eth_trans_type() does a skb_pull(, dev->hard_header_len), which is
inconsistent, since it interprets the pulled header as struct ethhdr,
which is ETH_HLEN long. It does not really make a difference currently,
since dev->hard_header_len == ETH_HLEN for all users. However, there are
(at least) two places in the tree which use their private copy of
eth_type_trans() since they have a non-standard dev->hard_header_len and
thus cannot use the generic function.
These two csets do not change the behavior for existing code, but remove
all references to dev->hard_header_len in net/ethernet/eth.c and thus make
these functions useful for other devices which have a non-standard
dev->hard_header_len ("ethernet over ISDN", e.g.).
--Kai
Pull from http://linux-isdn.bkbits.net/linux-2.5.net
(Merging changesets omitted for clarity)
-----------------------------------------------------------------------------
ChangeSet@1.635, 2002-09-29 13:00:13-05:00, kai@tp1.ruhr-uni-bochum.de
NET: Do not use dev->hard_header_len in eth_header()
The actual return value of eth_header() is never used, only its sign.
So it does not make a difference if we return dev->hard_header_len or
ETH_HLEN, but the latter makes more sense as that is the number of bytes
we added to the front of the frame.
For 99% of the drivers, dev->hard_header_len == ETH_HLEN, so no difference
at all, but if a driver actually needs additional headroom and thus
has dev->hard_header_len > ETH_HLEN, the process of building the ethernet
header should not care at all.
----------------------------------------------------------------------------
eth.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
-----------------------------------------------------------------------------
ChangeSet@1.636, 2002-09-29 13:08:19-05:00, kai@tp1.ruhr-uni-bochum.de
NET: Do not use dev->hard_header_len in eth_type_trans()
eth_type_trans() currently pulls dev->hard_header_len off a frame
passed to it, however always interpreting it as a ethernet header.
Grepping shows that it is only used on net devices where
dev->hard_header_len == ETH_HLEN. It makes more sense to actually
pull of ETH_HLEN for the header (it's treated as a struct of the length
anyway), not changing the behavior for the existing users but allowing
two places which had to use their private copies of eth_trans_type to
use the generic routine now.
One place is in drivers/net/hamachi.c and converted in this cset, the other
one is in the ISDN network code, patch will follow.
----------------------------------------------------------------------------
drivers/net/hamachi.c | 63 --------------------------------------------------
net/ethernet/eth.c | 2 -
2 files changed, 1 insertion(+), 64 deletions(-)
=============================================================================
unified diffs follow for reference
=============================================================================
-----------------------------------------------------------------------------
ChangeSet@1.635, 2002-09-29 13:00:13-05:00, kai@tp1.ruhr-uni-bochum.de
NET: Do not use dev->hard_header_len in eth_header()
The actual return value of eth_header() is never used, only its sign.
So it does not make a difference if we return dev->hard_header_len or
ETH_HLEN, but the latter makes more sense as that is the number of bytes
we added to the front of the frame.
For 99% of the drivers, dev->hard_header_len == ETH_HLEN, so no difference
at all, but if a driver actually needs additional headroom and thus
has dev->hard_header_len > ETH_HLEN, the process of building the ethernet
header should not care at all.
---------------------------------------------------------------------------
diff -Nru a/net/ethernet/eth.c b/net/ethernet/eth.c
--- a/net/ethernet/eth.c Sun Sep 29 13:10:35 2002
+++ b/net/ethernet/eth.c Sun Sep 29 13:10:35 2002
@@ -103,16 +103,16 @@
if (dev->flags & (IFF_LOOPBACK|IFF_NOARP))
{
memset(eth->h_dest, 0, dev->addr_len);
- return(dev->hard_header_len);
+ return ETH_HLEN;
}
if(daddr)
{
memcpy(eth->h_dest,daddr,dev->addr_len);
- return dev->hard_header_len;
+ return ETH_HLEN;
}
- return -dev->hard_header_len;
+ return -ETH_HLEN;
}
-----------------------------------------------------------------------------
ChangeSet@1.636, 2002-09-29 13:08:19-05:00, kai@tp1.ruhr-uni-bochum.de
NET: Do not use dev->hard_header_len in eth_type_trans()
eth_type_trans() currently pulls dev->hard_header_len off a frame
passed to it, however always interpreting it as a ethernet header.
Grepping shows that it is only used on net devices where
dev->hard_header_len == ETH_HLEN. It makes more sense to actually
pull of ETH_HLEN for the header (it's treated as a struct of the length
anyway), not changing the behavior for the existing users but allowing
two places which had to use their private copies of eth_trans_type to
use the generic routine now.
One place is in drivers/net/hamachi.c and converted in this cset, the other
one is in the ISDN network code, patch will follow.
---------------------------------------------------------------------------
diff -Nru a/drivers/net/hamachi.c b/drivers/net/hamachi.c
--- a/drivers/net/hamachi.c Sun Sep 29 13:10:36 2002
+++ b/drivers/net/hamachi.c Sun Sep 29 13:10:36 2002
@@ -1460,64 +1460,6 @@
spin_unlock(&hmp->lock);
}
-#ifdef TX_CHECKSUM
-/*
- * Copied from eth_type_trans(), with reduced header, since we don't
- * get it on RX, only on TX.
- */
-static unsigned short hamachi_eth_type_trans(struct sk_buff *skb,
- struct net_device *dev)
-{
- struct ethhdr *eth;
- unsigned char *rawp;
-
- skb->mac.raw=skb->data;
- skb_pull(skb,dev->hard_header_len-8); /* artificially enlarged on tx */
- eth= skb->mac.ethernet;
-
- if(*eth->h_dest&1)
- {
- if(memcmp(eth->h_dest,dev->broadcast, ETH_ALEN)==0)
- skb->pkt_type=PACKET_BROADCAST;
- else
- skb->pkt_type=PACKET_MULTICAST;
- }
-
- /*
- * This ALLMULTI check should be redundant by 1.4
- * so don't forget to remove it.
- *
- * Seems, you forgot to remove it. All silly devices
- * seems to set IFF_PROMISC.
- */
-
- else if(dev->flags&(IFF_PROMISC/*|IFF_ALLMULTI*/))
- {
- if(memcmp(eth->h_dest,dev->dev_addr, ETH_ALEN))
- skb->pkt_type=PACKET_OTHERHOST;
- }
-
- if (ntohs(eth->h_proto) >= 1536)
- return eth->h_proto;
-
- rawp = skb->data;
-
- /*
- * This is a magic hack to spot IPX packets. Older Novell breaks
- * the protocol design and runs IPX over 802.3 without an 802.2 LLC
- * layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This
- * won't work for fault tolerant netware but does for the rest.
- */
- if (*(unsigned short *)rawp == 0xFFFF)
- return htons(ETH_P_802_3);
-
- /*
- * Real 802.2 LLC
- */
- return htons(ETH_P_802_2);
-}
-#endif /* TX_CHECKSUM */
-
/* This routine is logically part of the interrupt handler, but seperated
for clarity and better register allocation. */
static int hamachi_rx(struct net_device *dev)
@@ -1622,12 +1564,7 @@
skb_put(skb = hmp->rx_skbuff[entry], pkt_len);
hmp->rx_skbuff[entry] = NULL;
}
-#ifdef TX_CHECKSUM
- /* account for extra TX hard_header bytes */
- skb->protocol = hamachi_eth_type_trans(skb, dev);
-#else
skb->protocol = eth_type_trans(skb, dev);
-#endif
#ifdef RX_CHECKSUM
diff -Nru a/net/ethernet/eth.c b/net/ethernet/eth.c
--- a/net/ethernet/eth.c Sun Sep 29 13:10:36 2002
+++ b/net/ethernet/eth.c Sun Sep 29 13:10:36 2002
@@ -161,7 +161,7 @@
unsigned char *rawp;
skb->mac.raw=skb->data;
- skb_pull(skb,dev->hard_header_len);
+ skb_pull(skb,ETH_HLEN);
eth= skb->mac.ethernet;
if(*eth->h_dest&1)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BK PATCH] Make eth.c independent of dev->hard_header_len
2002-09-29 18:34 [BK PATCH] Make eth.c independent of dev->hard_header_len Kai Germaschewski
@ 2002-09-29 18:45 ` Jeff Garzik
2002-09-29 19:09 ` Kai Germaschewski
2002-09-30 4:36 ` David S. Miller
1 sibling, 1 reply; 6+ messages in thread
From: Jeff Garzik @ 2002-09-29 18:45 UTC (permalink / raw)
To: Kai Germaschewski; +Cc: David S. Miller, netdev
Kai Germaschewski wrote:
> --- a/drivers/net/hamachi.c Sun Sep 29 13:10:36 2002
> +++ b/drivers/net/hamachi.c Sun Sep 29 13:10:36 2002
I would prefer to keep the code inside TX_CHECKSUM in hamachi.c, hoping
somebody will complete it, not rip it out :)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BK PATCH] Make eth.c independent of dev->hard_header_len
2002-09-29 18:45 ` Jeff Garzik
@ 2002-09-29 19:09 ` Kai Germaschewski
2002-09-30 0:56 ` David S. Miller
0 siblings, 1 reply; 6+ messages in thread
From: Kai Germaschewski @ 2002-09-29 19:09 UTC (permalink / raw)
To: Jeff Garzik; +Cc: David S. Miller, netdev
On Sun, 29 Sep 2002, Jeff Garzik wrote:
> Kai Germaschewski wrote:
> > --- a/drivers/net/hamachi.c Sun Sep 29 13:10:36 2002
> > +++ b/drivers/net/hamachi.c Sun Sep 29 13:10:36 2002
>
> I would prefer to keep the code inside TX_CHECKSUM in hamachi.c, hoping
> somebody will complete it, not rip it out :)
I didn't really rip out anything. I just removed code cut'n'pasted from
net/ethernet/eth.c (+ slight modification to handle dev->hard_header_len
!= ETH_HLEN), since after the change we can use the generic
eth_trans_type() directly. The TX checksum thing is not at all affected by
that piece of code.
--Kai
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BK PATCH] Make eth.c independent of dev->hard_header_len
2002-09-29 19:09 ` Kai Germaschewski
@ 2002-09-30 0:56 ` David S. Miller
2002-09-30 1:58 ` Jeff Garzik
0 siblings, 1 reply; 6+ messages in thread
From: David S. Miller @ 2002-09-30 0:56 UTC (permalink / raw)
To: kai-germaschewski; +Cc: jgarzik, netdev
From: Kai Germaschewski <kai-germaschewski@uiowa.edu>
Date: Sun, 29 Sep 2002 14:09:11 -0500 (CDT)
On Sun, 29 Sep 2002, Jeff Garzik wrote:
> Kai Germaschewski wrote:
> > --- a/drivers/net/hamachi.c Sun Sep 29 13:10:36 2002
> > +++ b/drivers/net/hamachi.c Sun Sep 29 13:10:36 2002
>
> I would prefer to keep the code inside TX_CHECKSUM in hamachi.c, hoping
> somebody will complete it, not rip it out :)
I didn't really rip out anything.
Jeff I think his patch is OK, please confirm.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BK PATCH] Make eth.c independent of dev->hard_header_len
2002-09-30 0:56 ` David S. Miller
@ 2002-09-30 1:58 ` Jeff Garzik
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Garzik @ 2002-09-30 1:58 UTC (permalink / raw)
To: David S. Miller; +Cc: kai-germaschewski, netdev
David S. Miller wrote:
> Jeff I think his patch is OK, please confirm.
yeah, ok with me
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BK PATCH] Make eth.c independent of dev->hard_header_len
2002-09-29 18:34 [BK PATCH] Make eth.c independent of dev->hard_header_len Kai Germaschewski
2002-09-29 18:45 ` Jeff Garzik
@ 2002-09-30 4:36 ` David S. Miller
1 sibling, 0 replies; 6+ messages in thread
From: David S. Miller @ 2002-09-30 4:36 UTC (permalink / raw)
To: kai-germaschewski; +Cc: netdev
From: Kai Germaschewski <kai-germaschewski@uiowa.edu>
Date: Sun, 29 Sep 2002 13:34:59 -0500 (CDT)
Pull from http://linux-isdn.bkbits.net/linux-2.5.net
I tried to pull, but:
? bk pull -n http://linux-isdn.bkbits.net/linux-2.5.net
Nothing to pull from http://linux-isdn.bkbits.net/linux-2.5.net
?
Are you sure the URL is correct?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2002-09-30 4:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2002-09-29 18:34 [BK PATCH] Make eth.c independent of dev->hard_header_len Kai Germaschewski
2002-09-29 18:45 ` Jeff Garzik
2002-09-29 19:09 ` Kai Germaschewski
2002-09-30 0:56 ` David S. Miller
2002-09-30 1:58 ` Jeff Garzik
2002-09-30 4:36 ` David S. Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).