netdev - Re: [PATCH] tg3: receive packets larger than MTU but smaller than 1500 bytes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20100223231522.GA850@xw6200.broadcom.net>
Date:	Tue, 23 Feb 2010 15:15:22 -0800
From:	"Matt Carlson" <mcarlson@...adcom.com>
To:	"Szilveszter Ordog" <slipszi@...il.com>
cc:	"Matthew Carlson" <mcarlson@...adcom.com>,
	"Michael Chan" <mchan@...adcom.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Ben Hutchings" <bhutchings@...arflare.com>,
	"Templin, Fred L" <Fred.L.Templin@...ing.com>
Subject: Re: [PATCH] tg3: receive packets larger than MTU but smaller
 than 1500 bytes

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@...il.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html