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@xxxxx, 2002-09-29 13:00:13-05:00, kai@xxxxxxxxxxxxxxxxxxxxxx
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@xxxxx, 2002-09-29 13:08:19-05:00, kai@xxxxxxxxxxxxxxxxxxxxxx
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@xxxxx, 2002-09-29 13:00:13-05:00, kai@xxxxxxxxxxxxxxxxxxxxxx
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@xxxxx, 2002-09-29 13:08:19-05:00, kai@xxxxxxxxxxxxxxxxxxxxxx
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)
|