* [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags. @ 2011-02-10 21:59 greearb 2011-02-10 21:59 ` [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets greearb ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: greearb @ 2011-02-10 21:59 UTC (permalink / raw) To: netdev; +Cc: Ben Greear From: Ben Greear <greearb@candelatech.com> 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 <greearb@candelatech.com> --- :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)) { -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets. 2011-02-10 21:59 [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags greearb @ 2011-02-10 21:59 ` greearb 2011-02-11 6:57 ` Eric Dumazet 2011-02-11 15:46 ` [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags Jeff Kirsher 2011-02-17 11:04 ` Jeff Kirsher 2 siblings, 1 reply; 15+ messages in thread From: greearb @ 2011-02-10 21:59 UTC (permalink / raw) To: netdev; +Cc: Ben Greear From: Ben Greear <greearb@candelatech.com> This allows user-space to send a '1500' MTU VLAN packet on a 1500 MTU ethernet frame. The extra 4 bytes of a VLAN header is not usually charged against the MTU when other parts of the network stack is transmitting vlans... Signed-off-by: Ben Greear <greearb@candelatech.com> --- :100644 100644 91cb1d7... ef7f378... M net/packet/af_packet.c net/packet/af_packet.c | 31 +++++++++++++++++++++++++++++-- 1 files changed, 29 insertions(+), 2 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 91cb1d7..ef7f378 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -466,7 +466,7 @@ retry: */ err = -EMSGSIZE; - if (len > dev->mtu + dev->hard_header_len) + if (len > dev->mtu + dev->hard_header_len + VLAN_HLEN) goto out_unlock; if (!skb) { @@ -497,6 +497,19 @@ retry: goto retry; } + if (len > (dev->mtu + dev->hard_header_len)) { + /* Earlier code assumed this would be a VLAN pkt, + * double-check this now that we have the actual + * packet in hand. + */ + struct ethhdr *ehdr; + skb_reset_mac_header(skb); + ehdr = eth_hdr(skb); + if (ehdr->h_proto != htons(ETH_P_8021Q)) { + err = -EMSGSIZE; + goto out_unlock; + } + } skb->protocol = proto; skb->dev = dev; @@ -1200,7 +1213,7 @@ static int packet_snd(struct socket *sock, } err = -EMSGSIZE; - if (!gso_type && (len > dev->mtu+reserve)) + if (!gso_type && (len > dev->mtu + reserve + ETH_HLEN)) goto out_unlock; err = -ENOBUFS; @@ -1225,6 +1238,20 @@ static int packet_snd(struct socket *sock, if (err < 0) goto out_free; + if (!gso_type && (len > dev->mtu + reserve)) { + /* Earlier code assumed this would be a VLAN pkt, + * double-check this now that we have the actual + * packet in hand. + */ + struct ethhdr *ehdr; + skb_reset_mac_header(skb); + ehdr = eth_hdr(skb); + if (ehdr->h_proto != htons(ETH_P_8021Q)) { + err = -EMSGSIZE; + goto out_free; + } + } + skb->protocol = proto; skb->dev = dev; skb->priority = sk->sk_priority; -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets. 2011-02-10 21:59 ` [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets greearb @ 2011-02-11 6:57 ` Eric Dumazet 2011-02-11 17:38 ` Ben Greear 0 siblings, 1 reply; 15+ messages in thread From: Eric Dumazet @ 2011-02-11 6:57 UTC (permalink / raw) To: greearb; +Cc: netdev Le jeudi 10 février 2011 à 13:59 -0800, greearb@candelatech.com a écrit : > From: Ben Greear <greearb@candelatech.com> > > This allows user-space to send a '1500' MTU VLAN packet on a > 1500 MTU ethernet frame. The extra 4 bytes of a VLAN header is > not usually charged against the MTU when other parts of the > network stack is transmitting vlans... > > Signed-off-by: Ben Greear <greearb@candelatech.com> > --- > :100644 100644 91cb1d7... ef7f378... M net/packet/af_packet.c > net/packet/af_packet.c | 31 +++++++++++++++++++++++++++++-- > 1 files changed, 29 insertions(+), 2 deletions(-) > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index 91cb1d7..ef7f378 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -466,7 +466,7 @@ retry: > */ > > err = -EMSGSIZE; > - if (len > dev->mtu + dev->hard_header_len) > + if (len > dev->mtu + dev->hard_header_len + VLAN_HLEN) > goto out_unlock; > > if (!skb) { > @@ -497,6 +497,19 @@ retry: > goto retry; > } > > + if (len > (dev->mtu + dev->hard_header_len)) { > + /* Earlier code assumed this would be a VLAN pkt, > + * double-check this now that we have the actual > + * packet in hand. > + */ > + struct ethhdr *ehdr; > + skb_reset_mac_header(skb); > + ehdr = eth_hdr(skb); > + if (ehdr->h_proto != htons(ETH_P_8021Q)) { > + err = -EMSGSIZE; > + goto out_unlock; This would leak skb. > + } > + } > > skb->protocol = proto; > skb->dev = dev; ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets. 2011-02-11 6:57 ` Eric Dumazet @ 2011-02-11 17:38 ` Ben Greear 2011-02-11 18:18 ` Eric Dumazet 0 siblings, 1 reply; 15+ messages in thread From: Ben Greear @ 2011-02-11 17:38 UTC (permalink / raw) To: Eric Dumazet; +Cc: netdev On 02/10/2011 10:57 PM, Eric Dumazet wrote: > Le jeudi 10 février 2011 à 13:59 -0800, greearb@candelatech.com a > écrit : >> From: Ben Greear<greearb@candelatech.com> >> >> This allows user-space to send a '1500' MTU VLAN packet on a >> 1500 MTU ethernet frame. The extra 4 bytes of a VLAN header is >> not usually charged against the MTU when other parts of the >> network stack is transmitting vlans... >> >> Signed-off-by: Ben Greear<greearb@candelatech.com> >> --- >> :100644 100644 91cb1d7... ef7f378... M net/packet/af_packet.c >> net/packet/af_packet.c | 31 +++++++++++++++++++++++++++++-- >> 1 files changed, 29 insertions(+), 2 deletions(-) >> >> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c >> index 91cb1d7..ef7f378 100644 >> --- a/net/packet/af_packet.c >> +++ b/net/packet/af_packet.c >> @@ -466,7 +466,7 @@ retry: >> */ >> >> err = -EMSGSIZE; >> - if (len> dev->mtu + dev->hard_header_len) >> + if (len> dev->mtu + dev->hard_header_len + VLAN_HLEN) >> goto out_unlock; >> >> if (!skb) { >> @@ -497,6 +497,19 @@ retry: >> goto retry; >> } >> >> + if (len> (dev->mtu + dev->hard_header_len)) { >> + /* Earlier code assumed this would be a VLAN pkt, >> + * double-check this now that we have the actual >> + * packet in hand. >> + */ >> + struct ethhdr *ehdr; >> + skb_reset_mac_header(skb); >> + ehdr = eth_hdr(skb); >> + if (ehdr->h_proto != htons(ETH_P_8021Q)) { >> + err = -EMSGSIZE; >> + goto out_unlock; > > This would leak skb. > >> + } >> + } >> >> skb->protocol = proto; >> skb->dev = dev; Can you double-check that? Seems to me that in that method the out_unlock falls through to the out_free case. The other method is the opposite, funny enough... Thanks, Ben > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Ben Greear <greearb@candelatech.com> Candela Technologies Inc http://www.candelatech.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets. 2011-02-11 17:38 ` Ben Greear @ 2011-02-11 18:18 ` Eric Dumazet 0 siblings, 0 replies; 15+ messages in thread From: Eric Dumazet @ 2011-02-11 18:18 UTC (permalink / raw) To: Ben Greear; +Cc: netdev Le vendredi 11 février 2011 à 09:38 -0800, Ben Greear a écrit : > Can you double-check that? Seems to me that in that method the out_unlock > falls through to the out_free case. The other method is the opposite, funny > enough... Oops you're right, sorry. Reading again your patch, I see you used one ETH_HLEN instead of VLAN_HLEN in packet_snd() ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags. 2011-02-10 21:59 [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags greearb 2011-02-10 21:59 ` [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets greearb @ 2011-02-11 15:46 ` Jeff Kirsher 2011-02-17 11:04 ` Jeff Kirsher 2 siblings, 0 replies; 15+ messages in thread From: Jeff Kirsher @ 2011-02-11 15:46 UTC (permalink / raw) To: greearb; +Cc: netdev On Thu, Feb 10, 2011 at 13:59, <greearb@candelatech.com> wrote: > From: Ben Greear <greearb@candelatech.com> > > 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 <greearb@candelatech.com> > --- > :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(-) > Thanks Ben, I have added the patch to my queue of igb patches. -- Cheers, Jeff ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags. 2011-02-10 21:59 [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags greearb 2011-02-10 21:59 ` [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets greearb 2011-02-11 15:46 ` [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags Jeff Kirsher @ 2011-02-17 11:04 ` Jeff Kirsher 2011-02-17 17:28 ` Ben Greear 2 siblings, 1 reply; 15+ messages in thread From: Jeff Kirsher @ 2011-02-17 11:04 UTC (permalink / raw) To: greearb; +Cc: netdev On Thu, Feb 10, 2011 at 13:59, <greearb@candelatech.com> wrote: > From: Ben Greear <greearb@candelatech.com> > > 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 <greearb@candelatech.com> > --- > :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. Thoughts? -- Cheers, Jeff ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags. 2011-02-17 11:04 ` Jeff Kirsher @ 2011-02-17 17:28 ` Ben Greear 2011-07-21 0:18 ` Jesse Gross 0 siblings, 1 reply; 15+ messages in thread From: Ben Greear @ 2011-02-17 17:28 UTC (permalink / raw) To: Jeff Kirsher; +Cc: netdev On 02/17/2011 03:04 AM, Jeff Kirsher wrote: > On Thu, Feb 10, 2011 at 13:59,<greearb@candelatech.com> wrote: >> From: Ben Greear<greearb@candelatech.com> >> >> 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<greearb@candelatech.com> >> --- >> :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? Thanks, Ben -- Ben Greear <greearb@candelatech.com> Candela Technologies Inc http://www.candelatech.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags. 2011-02-17 17:28 ` Ben Greear @ 2011-07-21 0:18 ` Jesse Gross 2011-07-21 0:27 ` Ben Greear 0 siblings, 1 reply; 15+ messages in thread From: Jesse Gross @ 2011-07-21 0:18 UTC (permalink / raw) To: Ben Greear; +Cc: Jeff Kirsher, netdev, Duyck, Alexander H On Thu, Feb 17, 2011 at 9:28 AM, Ben Greear <greearb@candelatech.com> wrote: > On 02/17/2011 03:04 AM, Jeff Kirsher wrote: >> >> On Thu, Feb 10, 2011 at 13:59,<greearb@candelatech.com> wrote: >>> >>> From: Ben Greear<greearb@candelatech.com> >>> >>> 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<greearb@candelatech.com> >>> --- >>> :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 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags. 2011-07-21 0:18 ` Jesse Gross @ 2011-07-21 0:27 ` Ben Greear 2011-07-21 1:21 ` Jeff Kirsher 0 siblings, 1 reply; 15+ messages in thread From: Ben Greear @ 2011-07-21 0:27 UTC (permalink / raw) To: Jesse Gross; +Cc: Jeff Kirsher, netdev, Duyck, Alexander H On 07/20/2011 05:18 PM, Jesse Gross wrote: > On Thu, Feb 17, 2011 at 9:28 AM, Ben Greear<greearb@candelatech.com> wrote: >> On 02/17/2011 03:04 AM, Jeff Kirsher wrote: >>> >>> On Thu, Feb 10, 2011 at 13:59,<greearb@candelatech.com> wrote: >>>> >>>> From: Ben Greear<greearb@candelatech.com> >>>> >>>> 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<greearb@candelatech.com> >>>> --- >>>> :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. Thanks, Ben -- Ben Greear <greearb@candelatech.com> Candela Technologies Inc http://www.candelatech.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags. 2011-07-21 0:27 ` Ben Greear @ 2011-07-21 1:21 ` Jeff Kirsher 2011-07-21 6:35 ` Alexander Duyck 0 siblings, 1 reply; 15+ messages in thread From: Jeff Kirsher @ 2011-07-21 1:21 UTC (permalink / raw) To: Ben Greear; +Cc: Jesse Gross, netdev@vger.kernel.org, Duyck, Alexander H [-- Attachment #1: Type: text/plain, Size: 3800 bytes --] 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<greearb@candelatech.com> wrote: > >> On 02/17/2011 03:04 AM, Jeff Kirsher wrote: > >>> > >>> On Thu, Feb 10, 2011 at 13:59,<greearb@candelatech.com> wrote: > >>>> > >>>> From: Ben Greear<greearb@candelatech.com> > >>>> > >>>> 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<greearb@candelatech.com> > >>>> --- > >>>> :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. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags. 2011-07-21 1:21 ` Jeff Kirsher @ 2011-07-21 6:35 ` Alexander Duyck 2011-07-21 21:44 ` Jesse Gross 2011-08-25 18:51 ` Ben Greear 0 siblings, 2 replies; 15+ messages in thread From: Alexander Duyck @ 2011-07-21 6:35 UTC (permalink / raw) To: jeffrey.t.kirsher Cc: Ben Greear, Jesse Gross, netdev@vger.kernel.org, Duyck, Alexander H On Wed, Jul 20, 2011 at 6:21 PM, Jeff Kirsher <jeffrey.t.kirsher@intel.com> 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<greearb@candelatech.com> wrote: >> >> On 02/17/2011 03:04 AM, Jeff Kirsher wrote: >> >>> >> >>> On Thu, Feb 10, 2011 at 13:59,<greearb@candelatech.com> wrote: >> >>>> >> >>>> From: Ben Greear<greearb@candelatech.com> >> >>>> >> >>>> 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<greearb@candelatech.com> >> >>>> --- >> >>>> :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. Thanks, Alex ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags. 2011-07-21 6:35 ` Alexander Duyck @ 2011-07-21 21:44 ` Jesse Gross 2011-08-25 18:51 ` Ben Greear 1 sibling, 0 replies; 15+ messages in thread From: Jesse Gross @ 2011-07-21 21:44 UTC (permalink / raw) To: Alexander Duyck Cc: jeffrey.t.kirsher, Ben Greear, netdev@vger.kernel.org, Duyck, Alexander H On Wed, Jul 20, 2011 at 11:35 PM, Alexander Duyck <alexander.duyck@gmail.com> wrote: > On Wed, Jul 20, 2011 at 6:21 PM, Jeff Kirsher > <jeffrey.t.kirsher@intel.com> 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<greearb@candelatech.com> wrote: >>> >> On 02/17/2011 03:04 AM, Jeff Kirsher wrote: >>> >>> >>> >>> On Thu, Feb 10, 2011 at 13:59,<greearb@candelatech.com> wrote: >>> >>>> >>> >>>> From: Ben Greear<greearb@candelatech.com> >>> >>>> >>> >>>> 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<greearb@candelatech.com> >>> >>>> --- >>> >>>> :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. Thanks Alex. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags. 2011-07-21 6:35 ` Alexander Duyck 2011-07-21 21:44 ` Jesse Gross @ 2011-08-25 18:51 ` Ben Greear 2011-08-25 23:31 ` Alexander Duyck 1 sibling, 1 reply; 15+ messages in thread From: Ben Greear @ 2011-08-25 18:51 UTC (permalink / raw) To: Alexander Duyck Cc: jeffrey.t.kirsher, Jesse Gross, netdev@vger.kernel.org, Duyck, Alexander H On 07/20/2011 11:35 PM, Alexander Duyck wrote: > On Wed, Jul 20, 2011 at 6:21 PM, Jeff Kirsher > <jeffrey.t.kirsher@intel.com> 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<greearb@candelatech.com> wrote: >>>>> On 02/17/2011 03:04 AM, Jeff Kirsher wrote: >>>>>> >>>>>> On Thu, Feb 10, 2011 at 13:59,<greearb@candelatech.com> wrote: >>>>>>> >>>>>>> From: Ben Greear<greearb@candelatech.com> >>>>>>> >>>>>>> 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<greearb@candelatech.com> >>>>>>> --- >>>>>>> :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 -- Ben Greear <greearb@candelatech.com> Candela Technologies Inc http://www.candelatech.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags. 2011-08-25 18:51 ` Ben Greear @ 2011-08-25 23:31 ` Alexander Duyck 0 siblings, 0 replies; 15+ messages in thread From: Alexander Duyck @ 2011-08-25 23:31 UTC (permalink / raw) To: Ben Greear Cc: Alexander Duyck, jeffrey.t.kirsher, Jesse Gross, netdev@vger.kernel.org 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 >> <jeffrey.t.kirsher@intel.com> 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<greearb@candelatech.com> wrote: >>>>>> On 02/17/2011 03:04 AM, Jeff Kirsher wrote: >>>>>>> >>>>>>> On Thu, Feb 10, 2011 at 13:59,<greearb@candelatech.com> wrote: >>>>>>>> >>>>>>>> From: Ben Greear<greearb@candelatech.com> >>>>>>>> >>>>>>>> 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<greearb@candelatech.com> >>>>>>>> --- >>>>>>>> :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 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-08-25 23:35 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-10 21:59 [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags greearb 2011-02-10 21:59 ` [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets greearb 2011-02-11 6:57 ` Eric Dumazet 2011-02-11 17:38 ` Ben Greear 2011-02-11 18:18 ` Eric Dumazet 2011-02-11 15:46 ` [PATCH 1/2] igb: Allow extra 4 bytes on RX for vlan tags Jeff Kirsher 2011-02-17 11:04 ` Jeff Kirsher 2011-02-17 17:28 ` Ben Greear 2011-07-21 0:18 ` Jesse Gross 2011-07-21 0:27 ` Ben Greear 2011-07-21 1:21 ` Jeff Kirsher 2011-07-21 6:35 ` Alexander Duyck 2011-07-21 21:44 ` Jesse Gross 2011-08-25 18:51 ` Ben Greear 2011-08-25 23:31 ` Alexander Duyck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).