From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Duyck Subject: Re: [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags. Date: Thu, 25 Aug 2011 16:31:56 -0700 Message-ID: <4E56DB6C.2020606@intel.com> References: <1297375149-18458-1-git-send-email-greearb@candelatech.com> <4D5D5AD8.5000802@candelatech.com> <4E277267.8090702@candelatech.com> <1311211304.2401.9.camel@jtkirshe-mobl> <4E569999.8050006@candelatech.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Alexander Duyck , jeffrey.t.kirsher@intel.com, Jesse Gross , "netdev@vger.kernel.org" To: Ben Greear Return-path: Received: from mga03.intel.com ([143.182.124.21]:57689 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751120Ab1HYXfh (ORCPT ); Thu, 25 Aug 2011 19:35:37 -0400 In-Reply-To: <4E569999.8050006@candelatech.com> Sender: netdev-owner@vger.kernel.org List-ID: On 08/25/2011 11:51 AM, Ben Greear wrote: > On 07/20/2011 11:35 PM, Alexander Duyck wrote: >> On Wed, Jul 20, 2011 at 6:21 PM, Jeff Kirsher >> wrote: >>> On Wed, 2011-07-20 at 17:27 -0700, Ben Greear wrote: >>>> On 07/20/2011 05:18 PM, Jesse Gross wrote: >>>>> On Thu, Feb 17, 2011 at 9:28 AM, Ben >>>>> Greear wrote: >>>>>> On 02/17/2011 03:04 AM, Jeff Kirsher wrote: >>>>>>> >>>>>>> On Thu, Feb 10, 2011 at 13:59, wrote: >>>>>>>> >>>>>>>> From: Ben Greear >>>>>>>> >>>>>>>> This allows the NIC to receive 1518 byte (not counting >>>>>>>> FCS) packets when MTU is 1500, thus allowing 1500 MTU >>>>>>>> VLAN frames to be received. Please note that no VLANs >>>>>>>> were actually configured on the NIC...it was just acting >>>>>>>> as pass-through device. >>>>>>>> >>>>>>>> Signed-off-by: Ben Greear >>>>>>>> --- >>>>>>>> :100644 100644 58c665b... 30c9cc6... M drivers/net/igb/igb_main.c >>>>>>>> drivers/net/igb/igb_main.c | 5 +++-- >>>>>>>> 1 files changed, 3 insertions(+), 2 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/net/igb/igb_main.c >>>>>>>> b/drivers/net/igb/igb_main.c >>>>>>>> index 58c665b..30c9cc6 100644 >>>>>>>> --- a/drivers/net/igb/igb_main.c >>>>>>>> +++ b/drivers/net/igb/igb_main.c >>>>>>>> @@ -2281,7 +2281,8 @@ static int __devinit igb_sw_init(struct >>>>>>>> igb_adapter >>>>>>>> *adapter) >>>>>>>> adapter->rx_itr_setting = IGB_DEFAULT_ITR; >>>>>>>> adapter->tx_itr_setting = IGB_DEFAULT_ITR; >>>>>>>> >>>>>>>> - adapter->max_frame_size = netdev->mtu + ETH_HLEN + >>>>>>>> ETH_FCS_LEN; >>>>>>>> + adapter->max_frame_size = (netdev->mtu + ETH_HLEN + >>>>>>>> ETH_FCS_LEN >>>>>>>> + + VLAN_HLEN); >>>>>>>> adapter->min_frame_size = ETH_ZLEN + ETH_FCS_LEN; >>>>>>>> >>>>>>>> spin_lock_init(&adapter->stats64_lock); >>>>>>>> @@ -4303,7 +4304,7 @@ static int igb_change_mtu(struct net_device >>>>>>>> *netdev, int new_mtu) >>>>>>>> { >>>>>>>> struct igb_adapter *adapter = netdev_priv(netdev); >>>>>>>> struct pci_dev *pdev = adapter->pdev; >>>>>>>> - int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN; >>>>>>>> + int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN + >>>>>>>> VLAN_HLEN; >>>>>>>> u32 rx_buffer_len, i; >>>>>>>> >>>>>>>> if ((new_mtu< 68) || (max_frame> >>>>>>>> MAX_JUMBO_FRAME_SIZE)) { >>>>>>> >>>>>>> While testing this patch, validation found that the patch >>>>>>> reduces the >>>>>>> maximum mtu size >>>>>>> by 4 bytes (reduces it from 9216 to 9212). This is not a >>>>>>> desired side >>>>>>> effect of this patch. >>>>>> >>>>>> You could add handling for that case and have it act as it used >>>>>> to when >>>>>> new_mtu is greater than 9212? >>>>>> >>>>>> I tested e1000e and it worked w/out hacking at 1500 MTU, so maybe >>>>>> check how it does it? >>>>> >>>>> I just wanted to bring this up again to see if any progress had been >>>>> made. We were looking at this driver and trying to figure out the >>>>> best way to convert it to use the new vlan model but I'm not familiar >>>> >>>> I've been watching :) >>>> >>>>> enough with the hardware to know. It seems that all of the other >>>>> Intel drivers unconditionally add space for the vlan tag to the >>>>> receive buffer (and would therefore have similar effects as this >>>>> patch), is there something different about this card? >>>>> >>>>> I believe that Alex was working on something in this area (in the >>>>> context of one of my patches from a long time ago) but I'm not sure >>>>> what came of that. >>>> >>>> Truth is, I don't really see why it's a problem to decrease the >>>> maximum MTU slightly in order to make it work with VLANs. >>>> >>>> I'm not sure if there is some way to make it work with VLANs >>>> and not decrease the maximum MTU. >>> >>> This was the reason this did not get accepted. I was looking into what >>> could be done so that we did not decease the maximum MTU, but I got >>> side-tracked and have not done anything on it in several months. >>> >> >> I can take a look at fixing this most likely tomorrow. I have some >> work planned for igb anyway over the next few days. >> >> Odds are it is just a matter of where the VLAN_HLEN is added. As I >> recall for our drivers the correct spot is in the setting of >> rx_buffer_len since that is the area more concerned with maximum >> receive frame size versus the mtu section which is more concerned with >> the transmit side of things. > > Did a patch for this ever get posted? I'll be happy to test it > if so... > > Thanks, > Ben > We haven't posted one yet. I have one written up but it is currently mixed in with a set of 30 patches that I am testing/cleaning up/formatting before submitting to our formal validation team. I will likely be submitting it to Jeff Kirsher sometime next week and the patches will probably be available a few weeks after that. Thanks, Alex