The patch titled
via-rhine: zero pad short packets on Rhine I ethernet cards
has been removed from the -mm tree. Its filename is
via-rhine-zero-pad-short-packets-on-rhine-i.patch
This patch was probably dropped from -mm because
it has now been merged into a subsystem tree or
into Linus's tree, or because it was folded into
its parent patch in the -mm tree.
From: Craig Brind <craigbrind@xxxxxxxxx>
Fixes Rhine I cards disclosing fragments of previously transmitted frames
in new transmissions.
Before transmission any socket buffer (skb) shorter than the ethernet
minimum length of 60 bytes was zero-padded within the skb. On Rhine I
cards the data can later be copied into an aligned transmission buffer
without copying this padding. This resulted in the transmission of the
frame with the extra bytes beyond the provided content leaking the previous
contents of this buffer on to the network.
Now the frame is only zero-padded once it reaches the buffer that it
will be directly transmitted from.
Testing:
I have recompiled and installed the patched via-rhine.ko module into
the stock SuSE 10.0 kernel. Frames shorter than 60 bytes are now
correctly zero-padded when captured on a separate host. The host has
run for a couple of days without incident. I see no unusual stats
reported by ifconfig, and no unusual log messages.
Rejected Alternative Design Decisions:
A More Cowardly Patch
Since I have no newer cards than Rhine I to test I considered simply
padding both the skb and the aligned buffer. This made for the
smallest source change (just the memset line) and provably only
effected Rhine I cards. I decided against it on efficiency grounds,
especially since the effected older cards would tend to be in older
machines. There is code inside skb_padto which potentially reallocates
and copies the entire buffer unessesarily if it's going to be copied
again later, as well as just the double memset costs.
Just Set skb->len to Include the Zeroes
Besides this still doing all of the extra effort described in the
"More Cowardly Patch", the skb implementation is complex. Adjusting
any of its struct members bypassing its approved API methods may work
now but may not in later versions.
A More Dangerous Patch
Moving the skb_padto inside the "else" rather than the introduction of
the "use_aligned_buffer" flag makes the code clearer. This would
require the assumption that skb_padto never reallocated the skb for
such small packets. I can't guarantee this assumption. I added
printk's and saw skb's of various sizes, sometimes as little as 128
bytes. Whilst 128 is long enough I can't be sure there won't be
shorter.
Signed-off-by: Craig Brind <craigbrind@xxxxxxxxx>
Cc: Roger Luethi <rl@xxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
---
drivers/net/via-rhine.c | 22 +++++++++++++++++++---
1 files changed, 19 insertions(+), 3 deletions(-)
diff -puN drivers/net/via-rhine.c~via-rhine-zero-pad-short-packets-on-rhine-i drivers/net/via-rhine.c
--- devel/drivers/net/via-rhine.c~via-rhine-zero-pad-short-packets-on-rhine-i 2006-04-20 01:01:26.000000000 -0700
+++ devel-akpm/drivers/net/via-rhine.c 2006-04-20 01:01:26.000000000 -0700
@@ -129,6 +129,7 @@
- Massive clean-up
- Rewrite PHY, media handling (remove options, full_duplex, backoff)
- Fix Tx engine race for good
+ - Craig Brind: Zero padded aligned buffers for short packets.
*/
@@ -1301,6 +1302,7 @@ static int rhine_start_tx(struct sk_buff
struct rhine_private *rp = netdev_priv(dev);
void __iomem *ioaddr = rp->base;
unsigned entry;
+ int use_aligned_buffer;
/* Caution: the write order is important here, set the field
with the "ownership" bits last. */
@@ -1308,16 +1310,27 @@ static int rhine_start_tx(struct sk_buff
/* Calculate the next Tx descriptor entry. */
entry = rp->cur_tx % TX_RING_SIZE;
- if (skb->len < ETH_ZLEN) {
+ /* Only pad skb if the card will copy directly from it. */
+ use_aligned_buffer = (rp->quirks & rqRhineI) &&
+ (((unsigned long)skb->data & 3) ||
+ skb_shinfo(skb)->nr_frags != 0 ||
+ skb->ip_summed == CHECKSUM_HW);
+
+ if ((skb->len < ETH_ZLEN) && !use_aligned_buffer) {
skb = skb_padto(skb, ETH_ZLEN);
if (skb == NULL)
return 0;
+
+ /* Padding may reallocate skb. */
+ use_aligned_buffer = (rp->quirks & rqRhineI) &&
+ (((unsigned long)skb->data & 3) ||
+ skb_shinfo(skb)->nr_frags != 0 ||
+ skb->ip_summed == CHECKSUM_HW);
}
rp->tx_skbuff[entry] = skb;
- if ((rp->quirks & rqRhineI) &&
- (((unsigned long)skb->data & 3) || skb_shinfo(skb)->nr_frags != 0 || skb->ip_summed == CHECKSUM_HW)) {
+ if (use_aligned_buffer) {
/* Must use alignment buffer. */
if (skb->len > PKT_BUF_SZ) {
/* packet too long, drop it */
@@ -1327,6 +1340,9 @@ static int rhine_start_tx(struct sk_buff
return 0;
}
skb_copy_and_csum_dev(skb, rp->tx_buf[entry]);
+ if (skb->len < ETH_ZLEN)
+ memset(rp->tx_buf[entry] + skb->len, 0,
+ ETH_ZLEN - skb->len);
rp->tx_skbuff_dma[entry] = 0;
rp->tx_ring[entry].addr = cpu_to_le32(rp->tx_bufs_dma +
(rp->tx_buf[entry] -
_
Patches currently in -mm which might be from craigbrind@xxxxxxxxx are
via-rhine-zero-pad-short-packets-on-rhine-i-ethernet-cards.patch
via-rhine-zero-pad-short-packets-on-rhine-i.patch
-
To unsubscribe from this list: send the line "unsubscribe mm-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
