Description of problem: http://marc.info/?l=linux-netdev&m=126210110408898&w=2 Instead of reverting Erics patch, we just set rx_buf_sz and copybreak to 16383. That should force the behavior we're after, and it can easily be tuned for hw that works properly in set_rx_max_buf. Signed-off-by: Neil Horman <nhorman> r8169.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index 60f96c4..cba3966 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; +/* + * copybreak default is set here so that we + * can copy frames rather than needing to constantly + * reallocate 4 page skbs + */ +static int rx_copybreak = 16383; static int use_dac; static struct { u32 msg_enable; @@ -3247,9 +3252,14 @@ 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 max_frame = dev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN; - - tp->rx_buf_sz = (max_frame > RX_BUF_SIZE) ? max_frame : RX_BUF_SIZE; + /* + * Note: Don't touch this. Some r8169 hw + * Can't deliver a proper frame length + * if rx filtering is enabled, so we need to + * disable it, which in turn means we need to + * be ready to receive maximally sized frames + */ + tp->rx_buf_sz = 16383; } static int rtl8169_open(struct net_device *dev) @@ -3383,7 +3393,7 @@ static u16 rtl_rw_cpluscmd(void __iomem *ioaddr) static void rtl_set_rx_max_size(void __iomem *ioaddr, unsigned int rx_buf_sz) { /* Low hurts. Let's disable the filtering. */ - RTL_W16(RxMaxSize, rx_buf_sz + 1); + RTL_W16(RxMaxSize, (rx_buf_sz == 16383) ? rx_buf_sz : rx_buf_sz + 1); } static void rtl8169_set_magic_reg(void __iomem *ioaddr, unsigned mac_version)
I actually just thought up a slight variation on this patch that will give users control over how this bug is handled. It takes davem's comments into account and I think at least partially satisfies his concerns over the performance impact of my current patch. I'll post it upstream and here in just a few minutes.
Created attachment 381759 [details] improved patch Ok, I've _just_ posted this version upstream. I don't think it completely satisfies davem's performance concerns, but it does mitigate them, in that with this version, users can optionally reset their ring buffer sizes such that performance is returned to previous levels (and the bug is re-introduced). If users can accept that (or use some other method to filter frames that are longer than they desire, I think this is likely the best we can do with the time we have.
This request was evaluated by Red Hat Product Management for inclusion in a Red Hat Enterprise Linux maintenance release. Product Management has requested further review of this request by Red Hat Engineering, for potential inclusion in a Red Hat Enterprise Linux Update release for currently deployed products. This request is not yet committed for inclusion in an Update release.
It seems that The upstream maintainer wandered away from this bug, As such Davem asked that I submit the RHEL fix as the official upstream solution again, so that it could be applied there. I've done so here: http://marc.info/?l=linux-netdev&m=126987867907375&w=1 So this should be closed as CURRENTRELEASE, given that the RHEL5 is now the upstream fix (or will be shortly)