Note: This bug is displayed in read-only format because the product is no longer active in Red Hat Bugzilla.
For bugs related to Red Hat Enterprise Linux 5 product line. The current stable release is 5.10. For Red Hat Enterprise Linux 6 and above, please visit Red Hat JIRA https://issues.redhat.com/secure/CreateIssue!default.jspa?pid=12332745 to report new issues.

Bug 552438

Summary: kernel: r8169: straighten out overlength frame detection (improved) [rhel-5.6]
Product: Red Hat Enterprise Linux 5 Reporter: Eugene Teo (Security Response) <eteo>
Component: kernelAssignee: Neil Horman <nhorman>
Status: CLOSED CURRENTRELEASE QA Contact: Network QE <network-qe>
Severity: medium Docs Contact:
Priority: medium    
Version: 5.5CC: davem, dhoward, eteo, jarod, jkurik, jpirko, jskrabal, kzhang, lgoncalv, lwang, mhlavink, nhorman, plyons
Target Milestone: rcKeywords: ZStream
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 2.6.18-186.el5 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 552439 (view as bug list) Environment:
Last Closed: 2010-03-29 16:06:57 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 552439, 552912, 552913, 554386    
Attachments:
Description Flags
improved patch none

Description Eugene Teo (Security Response) 2010-01-05 03:48:18 UTC
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)

Comment 3 Neil Horman 2010-01-05 13:25:58 UTC
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.

Comment 4 Neil Horman 2010-01-05 14:11:39 UTC
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.

Comment 5 RHEL Program Management 2010-01-06 14:21:00 UTC
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.

Comment 18 Neil Horman 2010-03-29 16:06:57 UTC
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)