Patchwork tg3: receive packets larger than MTU but smaller than 1500 bytes

login
register
mail settings
Submitter Szilveszter Ordog
Date Feb. 23, 2010, 11:56 a.m.
Message ID <ac67d9131002230356h7d293a5yba011698302f8c4a@mail.gmail.com>
Download mbox | patch
Permalink /patch/46059/
State Rejected
Delegated to: David Miller
Headers show

Comments

Szilveszter Ordog - Feb. 23, 2010, 11:56 a.m.
This allows for proper ICMP needs fragmentation responses on standard Ethernet
networks.

Signed-off-by: Szilveszter Ördög <slipszi@gmail.com>
---
 drivers/net/tg3.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Matt Carlson - Feb. 23, 2010, 11:15 p.m.
This isn't such a good idea, is it?  The tg3 driver tries very hard to
configure the hardware to the requirements specified by they kernel.  If
you break that link, then you reduce the effectiveness of the hardware's
filtering based on packet size and you interfere with the hardware's
ability to tally oversized frames.

This patch also makes assumptions about how large the driver's internal
buffer sizes are.

I saw the conversation you had with Ben Hutchings and Fred Templin.
Perhaps it is better to first decide if we should decouple the tx MTU
from the rx MTU.  If everyone agrees to that, then we can work on
implementing a proper driver interface to describe our intentions.

On Tue, Feb 23, 2010 at 03:56:26AM -0800, Szilveszter Ordog wrote:
> This allows for proper ICMP needs fragmentation responses on standard Ethernet
> networks.
> 
> Signed-off-by: Szilveszter ?rd?g <slipszi@gmail.com>
> ---
>  drivers/net/tg3.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> index 46a3f86..0c5027b 100644
> --- a/drivers/net/tg3.c
> +++ b/drivers/net/tg3.c
> @@ -93,6 +93,8 @@
>  #define TG3_MIN_MTU			60
>  #define TG3_MAX_MTU(tp)	\
>  	((tp->tg3_flags2 & TG3_FLG2_JUMBO_CAPABLE) ? 9000 : 1500)
> +#define TG3_MAX_RX_MTU(tp) \
> +	max(tp->dev->mtu, (unsigned int)ETH_DATA_LEN)
> 
>  /* These numbers seem to be hard coded in the NIC firmware somehow.
>   * You can't change the ring sizes, but you can change where you place
> @@ -4532,7 +4534,7 @@ static int tg3_rx(struct tg3 *tp, int budget)
> 
>  		skb->protocol = eth_type_trans(skb, tp->dev);
> 
> -		if (len > (tp->dev->mtu + ETH_HLEN) &&
> +		if (len > (TG3_MAX_RX_MTU(tp) + ETH_HLEN) &&
>  		    skb->protocol != htons(ETH_P_8021Q)) {
>  			dev_kfree_skb(skb);
>  			goto next_pkt;
> @@ -6979,7 +6981,7 @@ static int tg3_reset_hw(struct tg3 *tp, int reset_phy)
> 
>  	/* MTU + ethernet header + FCS + optional VLAN tag */
>  	tw32(MAC_RX_MTU_SIZE,
> -	     tp->dev->mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN);
> +	     TG3_MAX_RX_MTU(tp) + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN);
> 
>  	/* The slot time is changed by tg3_setup_phy if we
>  	 * run at gigabit with half duplex.
> -- 
> 1.6.3.3
> 

--
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

Patch

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 46a3f86..0c5027b 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -93,6 +93,8 @@ 
 #define TG3_MIN_MTU			60
 #define TG3_MAX_MTU(tp)	\
 	((tp->tg3_flags2 & TG3_FLG2_JUMBO_CAPABLE) ? 9000 : 1500)
+#define TG3_MAX_RX_MTU(tp) \
+	max(tp->dev->mtu, (unsigned int)ETH_DATA_LEN)

 /* These numbers seem to be hard coded in the NIC firmware somehow.
  * You can't change the ring sizes, but you can change where you place
@@ -4532,7 +4534,7 @@  static int tg3_rx(struct tg3 *tp, int budget)

 		skb->protocol = eth_type_trans(skb, tp->dev);

-		if (len > (tp->dev->mtu + ETH_HLEN) &&
+		if (len > (TG3_MAX_RX_MTU(tp) + ETH_HLEN) &&
 		    skb->protocol != htons(ETH_P_8021Q)) {
 			dev_kfree_skb(skb);
 			goto next_pkt;
@@ -6979,7 +6981,7 @@  static int tg3_reset_hw(struct tg3 *tp, int reset_phy)

 	/* MTU + ethernet header + FCS + optional VLAN tag */
 	tw32(MAC_RX_MTU_SIZE,
-	     tp->dev->mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN);
+	     TG3_MAX_RX_MTU(tp) + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN);

 	/* The slot time is changed by tg3_setup_phy if we
 	 * run at gigabit with half duplex.