When fixing the TSO support I noticed we just mask skb->gso_size with the MSSMask value and don't care about the consequences.
That seems... unsagacious. Instead of masking, WARN and drop the packet if the maximum is exceeded. And call netif_set_gso_max_size() so it shouldn't happen in the first place. Finally, Francois Romieu noticed that we didn't even have the right value for MSSMask anyway; it should be 0x7ff (11 bits) not 0xfff. Signed-off-by: David Woodhouse <david.woodho...@intel.com> --- Nice and simple and obvious, right? So why does it stop TSO from happening at all? When I call netif_set_gso_max_size() with a value of 0xaad (2733) or higher, I see TSO being used (with an MSS of 1214; this is just between a VM and its host; Legacy IP with an MTU of 1500). If I set a maximum of 0xaac or lower, TSO never gets used. Am I doing something wrong? Or is there a problem somewhere else? drivers/net/ethernet/realtek/8139cp.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c index 686334f..ae24d42 100644 --- a/drivers/net/ethernet/realtek/8139cp.c +++ b/drivers/net/ethernet/realtek/8139cp.c @@ -175,7 +175,7 @@ enum { LastFrag = (1 << 28), /* Final segment of a packet */ LargeSend = (1 << 27), /* TCP Large Send Offload (TSO) */ MSSShift = 16, /* MSS value position */ - MSSMask = 0xfff, /* MSS value: 11 bits */ + MSSMask = 0x7ff, /* MSS value: 11 bits */ TxError = (1 << 23), /* Tx error summary */ RxError = (1 << 20), /* Rx error summary */ IPCS = (1 << 18), /* Calculate IP checksum */ @@ -754,10 +754,16 @@ static netdev_tx_t cp_start_xmit (struct sk_buff *skb, eor = (entry == (CP_TX_RING_SIZE - 1)) ? RingEnd : 0; mss = skb_shinfo(skb)->gso_size; + if (mss > MSSMask) { + WARN_ONCE(1, "Net bug: GSO size %d too large for 8139CP\n", + mss); + goto out_dma_error; + } + opts2 = cpu_to_le32(cp_tx_vlan_tag(skb)); opts1 = DescOwn; if (mss) - opts1 |= LargeSend | ((mss & MSSMask) << MSSShift); + opts1 |= LargeSend | (mss << MSSShift); else if (skb->ip_summed == CHECKSUM_PARTIAL) { const struct iphdr *ip = ip_hdr(skb); if (ip->protocol == IPPROTO_TCP) @@ -1994,6 +2000,8 @@ static int cp_init_one (struct pci_dev *pdev, const struct pci_device_id *ent) dev->vlan_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO | NETIF_F_HIGHDMA; + netif_set_gso_max_size(dev, MSSMask); + rc = register_netdev(dev); if (rc) goto err_out_iomap; -- 2.4.3 -- David Woodhouse Open Source Technology Centre david.woodho...@intel.com Intel Corporation
smime.p7s
Description: S/MIME cryptographic signature