> Ok, third times the charm. Its been noted that both apporaches to fixing this
> bug are going to have major performance impacts, which is bad. Unless we can
> get a list of affected hardware to restrict the fix to certain NICS, I don't see
> how we can get around that. I have come up with this patch however, which I
> think at least partially addresses the performance concerns.
>
> A while back Eric submitted a patch for r8169 in which the proper
> allocated frame size was written to RXMaxSize to prevent the NIC from dmaing too
> much data. This was done in commit fdd7b4c3302c93f6833e338903ea77245eb510b4. A
> long time prior to that however, Francois posted
> 126fa4b9ca5d9d7cb7d46f779ad3bd3631ca387c, which expiclitly disabled the MaxSize
> setting due to the fact that the hardware behaved in odd ways when overlong
> frames were received on NIC's supported by this driver. This was mentioned in a
> security conference recently:
>
http://events.ccc.de/congress/2009/Fahrplan//events/3596.en.html
>
> It seems that if we can't enable frame size filtering, then, as Eric correctly
> noticed, we can find ourselves DMA-ing too much data to a buffer, causing
> corruption. As a result is seems that we are forced to allocate a frame which
> is ready to handle a maximally sized receive.
>
> This obviously has performance issues with it, so to mitigate that issue, this
> patch does two things:
>
> 1) Raises the copybreak value to the frame allocation size, which should force
> appropriately sized packets to get allocated on rx, rather than a full new 16k
> buffer.
>
> 2) This patch only disables frame filtering initially (i.e., during the NIC
> open), changing the MTU results in ring buffer allocation of a size in relation
> to the new mtu (along with a warning indicating that this is dangerous).
>
> Because of item (2), individuals who can't cope with the performance hit (or can
> otherwise filter frames to prevent the bug), or who have hardware they are sure
> is unaffected by this issue, can manually lower the copybreak and reset the mtu
> such that performance is restored easily.
>
> Signed-off-by: Neil Horman <nhorman@redhat.com>
>
>
>
> r8169.c | 29 ++++++++++++++++++++++++-----
> 1 file changed, 24 insertions(+), 5 deletions(-)
>
>
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index c403ce0..9aac49c 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -186,7 +186,12 @@ static struct pci_device_id rtl8169_pci_tbl[] = {
>
> MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl);
>
> -static int rx_copybreak = 200;
> +/*
> + * we set our copybreak very high so that we don't have
> + * to allocate 16k frames all the time (see note in
> + * rtl8169_open()
> + */
> +static int rx_copybreak = 16383;
> static int use_dac;
> static struct {
> u32 msg_enable;
> @@ -3240,9 +3245,13 @@ static void __devexit rtl8169_remove_one(struct pci_dev *pdev)
> }
>
> static void rtl8169_set_rxbufsize(struct rtl8169_private *tp,
> - struct net_device *dev)
> + unsigned int mtu)
> {
> - unsigned int max_frame = dev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
> + unsigned int max_frame = mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
> +
> + if (max_frame != 16383)
> + printk(KERN_WARNING "WARNING! Changing of MTU on this NIC"
> + "May lead to frame reception errors!\n");
>
> tp->rx_buf_sz = (max_frame > RX_BUF_SIZE) ? max_frame : RX_BUF_SIZE;
> }
> @@ -3254,7 +3263,17 @@ static int rtl8169_open(struct net_device *dev)
> int retval = -ENOMEM;
>
>
> - rtl8169_set_rxbufsize(tp, dev);
> + /*
> + * Note that we use a magic value here, its wierd I know
> + * its done because, some subset of rtl8169 hardware suffers from
> + * a problem in which frames received that are longer than
> + * the size set in RxMaxSize register return garbage sizes
> + * when received. To avoid this we need to turn off filtering,
> + * which is done by setting a value of 16383 in the RxMaxSize register
> + * and allocating 16k frames to handle the largest possible rx value
> + * thats what the magic math below does.
> + */
> + rtl8169_set_rxbufsize(tp, 16383 - VLAN_ETH_HLEN - ETH_FCS_LEN);
>
> /*
> * Rx and Tx desscriptors needs 256 bytes alignment.
> @@ -3907,7 +3926,7 @@ static int rtl8169_change_mtu(struct net_device *dev, int new_mtu)
>
> rtl8169_down(dev);
>
> - rtl8169_set_rxbufsize(tp, dev);
> + rtl8169_set_rxbufsize(tp, dev->mtu);
>
> ret = rtl8169_init_ring(dev);
> if (ret < 0)
> --
