igb: Allow extra 4 bytes on RX for vlan tags.
Subsystems (3)
PATCH 1/2 igb: Allow extra 4 bytes on RX for vlan tags.
From: Ben Greear <[email protected]>
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.
---
: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(-)
drivers/net/igb/igb_main.c +3 -2
@@ -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 )) {
Signature · 1 line
1.7.2.3
PATCH 2/2 network: Allow af_packet to transmit +4 bytes for VLAN packets.
From: Ben Greear <[email protected]>
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...
---
:100644 100644 91cb1d7... ef7f378... M net/packet/af_packet.c
net/packet/af_packet.c | 31 +++++++++++++++++++++++++++++-- 1 files changed, 29 insertions(+), 2 deletions(-)
net/packet/af_packet.c +29 -2
@@ -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;
Signature · 1 line
1.7.2.3
Re: [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets.
Le jeudi 10 février 2011 à 13:59 -0800, [email protected] a écrit :
Quoted 31 lines
From: Ben Greear <[email protected]>
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 <[email protected]>---
:100644 100644 91cb1d7... ef7f378... Mnet/packet/af_packet.cnet/packet/af_packet.c | 31 +++++++++++++++++++++++++++++-- 1 files changed, 29 insertions(+), 2 deletions(-)net/packet/af_packet.c +2
-1@@ -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)) {
net/packet/af_packet.c retry: · line 510
Quoted 10 lines
net/packet/af_packet.c +10
-0+/* 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.
Quoted 5 lines
net/packet/af_packet.c +2
-0+ } + } skb->protocol = proto; skb->dev = dev;
Re: [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets.
Quoted 46 lines · Eric Dumazet
Le jeudi 10 février 2011 à 13:59 -0800, [email protected] a écrit :
Quoted 31 lines
From: Ben Greear<[email protected]>
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<[email protected]>---
:100644 100644 91cb1d7... ef7f378... Mnet/packet/af_packet.cnet/packet/af_packet.c | 31 +++++++++++++++++++++++++++++-- 1 files changed, 29 insertions(+), 2 deletions(-)net/packet/af_packet.c +2
-1@@ -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)) {net/packet/af_packet.c retry: · line 510
Quoted 10 lines
net/packet/af_packet.c +10
-0+/* 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.
Quoted 5 lines
net/packet/af_packet.c retry: · line 515
Quoted 5 lines
net/packet/af_packet.c +2
-0+ } + } 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
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
Re: [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets.
Quoted 3 lines · Ben Greear
Can you double-check that? Seems to me that in that method the
out_unlockfalls through to theout_freecase. 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()
Quoted 8 lines · On Thu, Feb 10, 2011 at 13:59, <[email protected]> wrote:
From: Ben Greear <[email protected]>
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.
Quoted 6 lines
Signed-off-by Ben Greear <[email protected]>---
:100644 100644 58c665b... 30c9cc6... M drivers/net/igb/igb_main.cdrivers/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.
Signature · 2 lines
Cheers, Jeff
Quoted 29 lines · On Thu, Feb 10, 2011 at 13:59, <[email protected]> wrote:
From: Ben Greear <[email protected]>
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 <[email protected]>---
:100644 100644 58c665b... 30c9cc6... M drivers/net/igb/igb_main.cdrivers/net/igb/igb_main.c| 5 +++--1 files changed, 3 insertions(+), 2 deletions(-)drivers/net/igb/igb_main.c +2
-1@@ -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);
drivers/net/igb/igb_main.c static int igb_change_mtu(struct net_device *netdev, int new_mtu) · line 4310
Quoted 8 lines
drivers/net/igb/igb_main.c +1
-1@@ -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?
Signature · 2 lines
Cheers, Jeff
Quoted 37 lines · Jeff Kirsher
Quoted 36 lines · On Thu, Feb 10, 2011 at 13:59,<[email protected]> wrote:
From: Ben Greear<[email protected]>
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<[email protected]>---
:100644 100644 58c665b... 30c9cc6... Mdrivers/net/igb/igb_main.cdrivers/net/igb/igb_main.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)drivers/net/igb/igb_main.c +3
-2@@ -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;
Quoted 6 lines
drivers/net/igb/igb_main.c static int igb_change_mtu(struct net_device *netdev, int new_mtu) · line 4310
Quoted 1 line
drivers/net/igb/igb_main.c +0
-0if ((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
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
Quoted 49 lines · Ben Greear
Quoted 41 lines · Jeff Kirsher
Quoted 39 lines · On Thu, Feb 10, 2011 at 13:59,<[email protected]> wrote:
From: Ben Greear<[email protected]>
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<[email protected]>---
:100644 100644 58c665b... 30c9cc6... M drivers/net/igb/igb_main.cdrivers/net/igb/igb_main.c| 5 +++--1 files changed, 3 insertions(+), 2 deletions(-)drivers/net/igb/igb_main.c +2
-1@@ -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);
drivers/net/igb/igb_main.c +1
-1@@ -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;Quoted 6 lines
Quoted 1 line
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.
Quoted 5 lines
You could add handling for that case and have it act as it used to when
new_mtuis 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.
Quoted 53 lines · Jesse Gross
Quoted 52 lines · Ben Greear
Quoted 41 lines · Jeff Kirsher
Quoted 39 lines · On Thu, Feb 10, 2011 at 13:59,<[email protected]> wrote:
From: Ben Greear<[email protected]>
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<[email protected]>---
:100644 100644 58c665b... 30c9cc6... Mdrivers/net/igb/igb_main.cdrivers/net/igb/igb_main.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)drivers/net/igb/igb_main.c +2
-1@@ -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);drivers/net/igb/igb_main.c +1
-1@@ -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;Quoted 6 lines
Quoted 1 line
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_mtuis greater than 9212?
Quoted 6 lines
Quoted 2 lines
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 :)
Quoted 8 lines
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
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
Quoted 72 lines · Ben Greear
Quoted 53 lines · Jesse Gross
Quoted 52 lines · Ben Greear
Quoted 41 lines · Jeff Kirsher
Quoted 39 lines · On Thu, Feb 10, 2011 at 13:59,<[email protected]> wrote:
From: Ben Greear<[email protected]>
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<[email protected]>---
:100644 100644 58c665b... 30c9cc6... Mdrivers/net/igb/igb_main.cdrivers/net/igb/igb_main.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)drivers/net/igb/igb_main.c +2
-1@@ -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);drivers/net/igb/igb_main.c +1
-1@@ -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;Quoted 6 lines
Quoted 1 line
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_mtuis greater than 9212?Quoted 6 lines
Quoted 2 lines
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 :)
Quoted 8 lines
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.
Quoted 5 lines
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.
Quoted 76 lines · Jeff Kirsher
Quoted 75 lines · Ben Greear
Quoted 53 lines · Jesse Gross
Quoted 52 lines · Ben Greear
Quoted 41 lines · Jeff Kirsher
Quoted 39 lines · On Thu, Feb 10, 2011 at 13:59,<[email protected]> wrote:
From: Ben Greear<[email protected]>
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<[email protected]>---
:100644 100644 58c665b... 30c9cc6... M drivers/net/igb/igb_main.cdrivers/net/igb/igb_main.c| 5 +++--1 files changed, 3 insertions(+), 2 deletions(-)drivers/net/igb/igb_main.c +2
-1@@ -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);drivers/net/igb/igb_main.c +1
-1@@ -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;Quoted 6 lines
Quoted 1 line
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_mtuis greater than 9212?Quoted 6 lines
Quoted 2 lines
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 :)
Quoted 8 lines
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.
Quoted 7 lines
Quoted 2 lines
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
Quoted 86 lines · Alexander Duyck
Quoted 76 lines · Jeff Kirsher
Quoted 75 lines · Ben Greear
Quoted 53 lines · Jesse Gross
Quoted 52 lines · Ben Greear
Quoted 41 lines · Jeff Kirsher
Quoted 39 lines · On Thu, Feb 10, 2011 at 13:59,<[email protected]> wrote:
From: Ben Greear<[email protected]>
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<[email protected]>---
:100644 100644 58c665b... 30c9cc6... M drivers/net/igb/igb_main.cdrivers/net/igb/igb_main.c| 5 +++--1 files changed, 3 insertions(+), 2 deletions(-)drivers/net/igb/igb_main.c +2
-1@@ -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);drivers/net/igb/igb_main.c +1
-1@@ -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;Quoted 6 lines
Quoted 1 line
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_mtuis greater than 9212?Quoted 6 lines
Quoted 2 lines
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 :)
Quoted 8 lines
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.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 8 lines
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_HLENis added. As I recall for our drivers the correct spot is in the setting ofrx_buffer_lensince 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.
Quoted 86 lines · Alexander Duyck
Quoted 76 lines · Jeff Kirsher
Quoted 75 lines · Ben Greear
Quoted 53 lines · Jesse Gross
Quoted 52 lines · Ben Greear
Quoted 41 lines · Jeff Kirsher
Quoted 39 lines · On Thu, Feb 10, 2011 at 13:59,<[email protected]> wrote:
From: Ben Greear<[email protected]>
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<[email protected]>---
:100644 100644 58c665b... 30c9cc6... Mdrivers/net/igb/igb_main.cdrivers/net/igb/igb_main.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)drivers/net/igb/igb_main.c +2
-1@@ -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);drivers/net/igb/igb_main.c +1
-1@@ -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;Quoted 6 lines
Quoted 1 line
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_mtuis greater than 9212?Quoted 6 lines
Quoted 2 lines
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 :)
Quoted 8 lines
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.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 8 lines
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_HLENis added. As I recall for our drivers the correct spot is in the setting ofrx_buffer_lensince 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
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
Quoted 101 lines · Ben Greear
Quoted 91 lines · Alexander Duyck
Quoted 81 lines · Jeff Kirsher
Quoted 80 lines · Ben Greear
Quoted 58 lines · Jesse Gross
Quoted 57 lines · Ben Greear
Quoted 45 lines · Jeff Kirsher
Quoted 43 lines · On Thu, Feb 10, 2011 at 13:59,<[email protected]> wrote:
From: Ben Greear<[email protected]>
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<[email protected]>---
:100644 100644 58c665b... 30c9cc6... Mdrivers/net/igb/igb_main.cdrivers/net/igb/igb_main.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)drivers/net/igb/igb_main.c +0
-0@@ -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_LENdrivers/net/igb/igb_main.c +2
-1+ + 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;Quoted 7 lines
Quoted 2 lines
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_mtuis greater than 9212?Quoted 6 lines
Quoted 2 lines
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 :)
Quoted 8 lines
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.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 8 lines
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_HLENis added. As I recall for our drivers the correct spot is in the setting ofrx_buffer_lensince 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.
Quoted 6 lines
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