[RFC/PATCH] ipg: add jumbo frame support kconfig option

[RFC/PATCH] ipg: add jumbo frame support kconfig option

[Pekka J Enberg]

[ Sorry for the duplicate. I typoed Francois' email address. ]

From: Pekka Enberg <penberg@cs.helsinki.fi>

Convert the internal JUMBO_FRAME #ifdef to CONFIG_IP1000_JUMBO_FRAME proper and
fix compilation errors.

Cc: Francois Romieu <romieu@fr.zoreil.com>
Cc: Sorbica Shieh <sorbica@icplus.com.tw>
Cc: Jesse Huang <jesse@icplus.com.tw>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---
 drivers/net/Kconfig |    8 ++++++++
 drivers/net/ipg.c   |   21 ++++++++++-----------
 drivers/net/ipg.h   |   11 ++++++-----
 3 files changed, 24 insertions(+), 16 deletions(-)

Index: linux-2.6/drivers/net/Kconfig
===================================================================
--- linux-2.6.orig/drivers/net/Kconfig
+++ linux-2.6/drivers/net/Kconfig
@@ -2029,6 +2029,14 @@ config IP1000
 	  To compile this driver as a module, choose M here: the module
 	  will be called ipg.  This is recommended.
 
+config IP1000_JUMBO_FRAME
+	bool "Support for jumbo frames (EXPERIMENTAL)"
+	depends on IP1000 && EXPERIMENTAL
+	help
+	  This option enables jumbo frame support for the IP1000 driver.
+
+	  If in doubt, say N.
+
 config IGB
        tristate "Intel(R) 82575 PCI-Express Gigabit Ethernet support"
        depends on PCI
Index: linux-2.6/drivers/net/ipg.c
===================================================================
--- linux-2.6.orig/drivers/net/ipg.c
+++ linux-2.6/drivers/net/ipg.c
@@ -42,7 +42,6 @@
 #define ipg_r16(reg)		ioread16(ioaddr + (reg))
 #define ipg_r8(reg)		ioread8(ioaddr + (reg))
 
-#define JUMBO_FRAME_4k_ONLY
 enum {
 	netdev_io_size = 128
 };
@@ -1079,7 +1078,7 @@ static int ipg_nic_rxrestore(struct net_
 	return 0;
 }
 
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 
 /* use jumboindex and jumbosize to control jumbo frame status
  * initial status is jumboindex=-1 and jumbosize=0
@@ -1274,7 +1273,7 @@ static void ipg_nic_rx_with_end(struct n
 
 			framelen = le64_to_cpu(rxfd->rfs) & IPG_RFS_RXFRAMELEN;
 
-			endframeLen = framelen - jumbo->current_size;
+			endframelen = framelen - jumbo->current_size;
 			/*
 			if (framelen > IPG_RXFRAG_SIZE)
 				framelen=IPG_RXFRAG_SIZE;
@@ -1282,8 +1281,8 @@ static void ipg_nic_rx_with_end(struct n
 			if (framelen > IPG_RXSUPPORT_SIZE)
 				dev_kfree_skb_irq(jumbo->skb);
 			else {
-				memcpy(skb_put(jumbo->skb, endframeLen),
-				       skb->data, endframeLen);
+				memcpy(skb_put(jumbo->skb, endframelen),
+				       skb->data, endframelen);
 
 				jumbo->skb->protocol =
 				    eth_type_trans(jumbo->skb, dev);
@@ -1355,16 +1354,16 @@ static int ipg_nic_rx(struct net_device 
 
 		switch (ipg_nic_rx_check_frame_type(dev)) {
 		case FRAME_WITH_START_WITH_END:
-			ipg_nic_rx_with_start_and_end(dev, tp, rxfd, entry);
+			ipg_nic_rx_with_start_and_end(dev, sp, rxfd, entry);
 			break;
 		case FRAME_WITH_START:
-			ipg_nic_rx_with_start(dev, tp, rxfd, entry);
+			ipg_nic_rx_with_start(dev, sp, rxfd, entry);
 			break;
 		case FRAME_WITH_END:
-			ipg_nic_rx_with_end(dev, tp, rxfd, entry);
+			ipg_nic_rx_with_end(dev, sp, rxfd, entry);
 			break;
 		case FRAME_NO_START_NO_END:
-			ipg_nic_rx_no_start_no_end(dev, tp, rxfd, entry);
+			ipg_nic_rx_no_start_no_end(dev, sp, rxfd, entry);
 			break;
 		}
 	}
@@ -1595,7 +1594,7 @@ static irqreturn_t ipg_interrupt_handler
 
 	IPG_DEBUG_MSG("_interrupt_handler\n");
 
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 	ipg_nic_rxrestore(dev);
 #endif
 	spin_lock(&sp->lock);
@@ -1807,7 +1806,7 @@ static int ipg_nic_open(struct net_devic
 	if (ipg_config_autoneg(dev) < 0)
 		printk(KERN_INFO "%s: Auto-negotiation error.\n", dev->name);
 
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 	/* initialize JUMBO Frame control variable */
 	sp->jumbo.found_start = 0;
 	sp->jumbo.current_size = 0;
Index: linux-2.6/drivers/net/ipg.h
===================================================================
--- linux-2.6.orig/drivers/net/ipg.h
+++ linux-2.6/drivers/net/ipg.h
@@ -536,7 +536,7 @@ enum ipg_regs {
  */
 #define		IPG_FRAMESBETWEENTXDMACOMPLETES 0x1
 
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 
 # ifdef JUMBO_FRAME_SIZE_2K
 # define JUMBO_FRAME_SIZE 2048
@@ -575,6 +575,7 @@ enum ipg_regs {
 #         define __IPG_RXFRAG_SIZE 4088
 #         else
 #         define JUMBO_FRAME_SIZE 4096
+#         define __IPG_RXFRAG_SIZE 4088
 #         endif
 #        endif
 #       endif
@@ -589,7 +590,7 @@ enum ipg_regs {
 /* Size of allocated received buffers. Nominally 0x0600.
  * Define larger if expecting jumbo frames.
  */
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 /* IPG_TXFRAG_SIZE must <= 0x2b00, or TX will crash */
 #define		IPG_TXFRAG_SIZE		JUMBO_FRAME_SIZE
 #endif
@@ -597,7 +598,7 @@ enum ipg_regs {
 /* Size of allocated received buffers. Nominally 0x0600.
  * Define larger if expecting jumbo frames.
  */
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 /* 4088 = 4096 - 8 */
 #define		IPG_RXFRAG_SIZE		__IPG_RXFRAG_SIZE
 #define     IPG_RXSUPPORT_SIZE   IPG_MAX_RXFRAME_SIZE
@@ -607,7 +608,7 @@ enum ipg_regs {
 #endif
 
 /* IPG_MAX_RXFRAME_SIZE <= IPG_RXFRAG_SIZE */
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 #define		IPG_MAX_RXFRAME_SIZE		JUMBO_FRAME_SIZE
 #else
 #define		IPG_MAX_RXFRAME_SIZE		0x0600
@@ -786,7 +787,7 @@ struct ipg_nic_private {
 	unsigned int tx_dirty;
 	unsigned int rx_current;
 	unsigned int rx_dirty;
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 	struct ipg_jumbo jumbo;
 #endif
 	unsigned int rx_buf_sz;

Re: [RFC/PATCH] ipg: add jumbo frame support kconfig option

[Stephen Hemminger]

> From: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> Convert the internal JUMBO_FRAME #ifdef to CONFIG_IP1000_JUMBO_FRAME proper and
> fix compilation errors.
> 
> Cc: Francois Romieu <romieu@fr.zoreil.com>
> Cc: Sorbica Shieh <sorbica@icplus.com.tw>
> Cc: Jesse Huang <jesse@icplus.com.tw>
> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

Please don't do this as a config option. Config options seem like a good idea
to developers but are difficult for distributions.  Either support jumbo
frames by default or not at all.

Re: [RFC/PATCH] ipg: add jumbo frame support kconfig option

[Jeff Garzik]

Stephen Hemminger wrote:
>> From: Pekka Enberg <penberg@cs.helsinki.fi>
>>
>> Convert the internal JUMBO_FRAME #ifdef to CONFIG_IP1000_JUMBO_FRAME proper and
>> fix compilation errors.
>>
>> Cc: Francois Romieu <romieu@fr.zoreil.com>
>> Cc: Sorbica Shieh <sorbica@icplus.com.tw>
>> Cc: Jesse Huang <jesse@icplus.com.tw>
>> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> Please don't do this as a config option. Config options seem like a good idea
> to developers but are difficult for distributions.  Either support jumbo
> frames by default or not at all.

Agreed, but to be more specific...    always compile in jumbo frame 
support.  You don't have to enable it at runtime by default, though.

	Jeff

Re: [RFC/PATCH] ipg: add jumbo frame support kconfig option

[Pekka J Enberg]

Hi,

Stephen Hemminger wrote:
> > Please don't do this as a config option. Config options seem like a good
> > idea to developers but are difficult for distributions.  Either 
> > support jumbo frames by default or not at all.

On Tue, 26 Feb 2008, Jeff Garzik wrote:
> Agreed, but to be more specific... always compile in jumbo frame 
> support. You don't have to enable it at runtime by default, though.

Ok, so a sysfs attribute would be sufficient, I suppose? I don't have the 
hardware though, so I don't even know if the jumbo frame code actually 
works at this point hence the RFC. So I'll wait for someone to test this 
before sending a new patch. Thanks for the review!

			Pekka

Re: [RFC/PATCH] ipg: add jumbo frame support kconfig option

[Pekka J Enberg]

On Wed, 27 Feb 2008, Pekka J Enberg wrote:
> Ok, so a sysfs attribute would be sufficient, I suppose?

s/sysfs attribute/module parameter/

Sorry, too early in the morning for me.

Re: [RFC/PATCH] ipg: add jumbo frame support kconfig option

[Jeff Garzik]

Pekka J Enberg wrote:
> On Wed, 27 Feb 2008, Pekka J Enberg wrote:
>> Ok, so a sysfs attribute would be sufficient, I suppose?
> 
> s/sysfs attribute/module parameter/

You could do it that way, but really, a non-jumbo default MTU should be 
sufficient.

The user must take proactive action to change the MTU to jumbo at that 
point...  that way its far more likely to get tested, while being 
non-default, too.

	Jeff

Re: [RFC/PATCH] ipg: add jumbo frame support kconfig option

[Andrew Savchenko]

[-- Attachment #1: Type: text/plain, Size: 875 bytes --]

Hi,

On Sunday 22 June 2008 13:23, Pekka J Enberg wrote:
> On Fri, 20 Jun 2008, Stephen Hemminger wrote:
> > The code should enable/disable anything it needs based on the
> > MTU size in the change_mtu call. If the driver needs to
> > reallocate buffers, change receiver settings, make sacrifices
> > to deities, or phone home that is the place to do it. In other
> > words no compile or config option fro MTU settings please.
>
> Right, so how about something like this instead? Andrew, can you
> test the following patch? You need all the other patches from
> the previous series modulo the last patch. Thanks!

Ok, I replaced old ipg-make-jumbo-frame-support-configurable with 
newer version. It works fine, I tuned MTU in the range from 68 to 
10240 bytes )).

Thanks for the patch, awaiting for it in the main kernel tree.

Sincerely yours,
Andrew

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC/PATCH] ipg: add jumbo frame support kconfig option

[Pekka J Enberg]

On Fri, 20 Jun 2008, Stephen Hemminger wrote:
> The code should enable/disable anything it needs based on the MTU size
> in the change_mtu call. If the driver needs to reallocate buffers, change receiver
> settings, make sacrifices to deities, or phone home that is the place to do it.
> In other words no compile or config option fro MTU settings please.

Right, so how about something like this instead? Andrew, can you test the 
following patch? You need all the other patches from the previous series 
modulo the last patch. Thanks!

		Pekka

Subject: [PATCH] ipg: run-time configurable jumbo frame support
From: Pekka Enberg <penberg@cs.helsinki.fi>

Make jumbo frame support configurable via ifconfig mtu option as suggested by
Stephen Hemminger.

Cc: Stephen Hemminger <stephen.hemminger@vyatta.com>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---
 drivers/net/ipg.c |   42 ++++++++++++++++++++------
 drivers/net/ipg.h |   85 ------------------------------------------------------
 2 files changed, 32 insertions(+), 95 deletions(-)

Index: linux-2.6/drivers/net/ipg.c
===================================================================
--- linux-2.6.orig/drivers/net/ipg.c	2008-06-22 11:55:53.000000000 +0300
+++ linux-2.6/drivers/net/ipg.c	2008-06-22 12:15:45.000000000 +0300
@@ -42,7 +42,6 @@
 #define ipg_r16(reg)		ioread16(ioaddr + (reg))
 #define ipg_r8(reg)		ioread8(ioaddr + (reg))
 
-#define JUMBO_FRAME_4k_ONLY
 enum {
 	netdev_io_size = 128
 };
@@ -55,6 +54,14 @@
 MODULE_LICENSE("GPL");
 
 /*
+ * Defaults
+ */
+#define IPG_MAX_RXFRAME_SIZE	0x0600
+#define IPG_RXFRAG_SIZE		0x0600
+#define IPG_RXSUPPORT_SIZE	0x0600
+#define IPG_IS_JUMBO		false
+
+/*
  * Variable record -- index by leading revision/length
  * Revision/Length(=N*4), Address1, Data1, Address2, Data2,...,AddressN,DataN
  */
@@ -1805,9 +1812,6 @@
 	sp->jumbo.current_size = 0;
 	sp->jumbo.skb = NULL;
 
-	if (IPG_TXFRAG_SIZE)
-		dev->mtu = IPG_TXFRAG_SIZE;
-
 	/* Enable transmit and receive operation of the IPG. */
 	ipg_w32((ipg_r32(MAC_CTRL) | IPG_MC_RX_ENABLE | IPG_MC_TX_ENABLE) &
 		 IPG_MC_RSVD_MASK, MAC_CTRL);
@@ -2116,6 +2120,7 @@
 static int ipg_nic_change_mtu(struct net_device *dev, int new_mtu)
 {
 	struct ipg_nic_private *sp = netdev_priv(dev);
+	int err;
 
 	/* Function to accomodate changes to Maximum Transfer Unit
 	 * (or MTU) of IPG NIC. Cannot use default function since
@@ -2124,16 +2129,33 @@
 
 	IPG_DEBUG_MSG("_nic_change_mtu\n");
 
-	/* Check that the new MTU value is between 68 (14 byte header, 46
-	 * byte payload, 4 byte FCS) and IPG_MAX_RXFRAME_SIZE, which
-	 * corresponds to the MAXFRAMESIZE register in the IPG.
+	/*
+	 * Check that the new MTU value is between 68 (14 byte header, 46 byte
+	 * payload, 4 byte FCS) and 10 KB, which is the largest supported MTU.
 	 */
-	if ((new_mtu < 68) || (new_mtu > sp->max_rxframe_size))
+	if (new_mtu < 68 || new_mtu > 10240)
 		return -EINVAL;
 
+	err = ipg_nic_stop(dev);
+	if (err)
+		return err;
+
 	dev->mtu = new_mtu;
 
-	return 0;
+	sp->max_rxframe_size = new_mtu;
+
+	sp->rxfrag_size = new_mtu;
+	if (sp->rxfrag_size > 4088)
+		sp->rxfrag_size = 4088;
+
+	sp->rxsupport_size = sp->max_rxframe_size;
+
+	if (new_mtu > 0x0600)
+		sp->is_jumbo = true;
+	else
+		sp->is_jumbo = false;
+
+	return ipg_nic_open(dev);
 }
 
 static int ipg_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
@@ -2238,7 +2260,7 @@
 	spin_lock_init(&sp->lock);
 	mutex_init(&sp->mii_mutex);
 
-	sp->is_jumbo = IPG_JUMBO;
+	sp->is_jumbo = IPG_IS_JUMBO;
 	sp->rxfrag_size = IPG_RXFRAG_SIZE;
 	sp->rxsupport_size = IPG_RXSUPPORT_SIZE;
 	sp->max_rxframe_size = IPG_MAX_RXFRAME_SIZE;
Index: linux-2.6/drivers/net/ipg.h
===================================================================
--- linux-2.6.orig/drivers/net/ipg.h	2008-06-22 11:55:53.000000000 +0300
+++ linux-2.6/drivers/net/ipg.h	2008-06-22 11:55:54.000000000 +0300
@@ -536,91 +536,6 @@
  */
 #define		IPG_FRAMESBETWEENTXDMACOMPLETES 0x1
 
-#ifdef JUMBO_FRAME
-# define IPG_JUMBO true
-#else
-# define IPG_JUMBO false
-#endif
-
-#ifdef JUMBO_FRAME
-
-# ifdef JUMBO_FRAME_SIZE_2K
-# define JUMBO_FRAME_SIZE 2048
-# define __IPG_RXFRAG_SIZE 2048
-# else
-#  ifdef JUMBO_FRAME_SIZE_3K
-#  define JUMBO_FRAME_SIZE 3072
-#  define __IPG_RXFRAG_SIZE 3072
-#  else
-#   ifdef JUMBO_FRAME_SIZE_4K
-#   define JUMBO_FRAME_SIZE 4096
-#   define __IPG_RXFRAG_SIZE 4088
-#   else
-#    ifdef JUMBO_FRAME_SIZE_5K
-#    define JUMBO_FRAME_SIZE 5120
-#    define __IPG_RXFRAG_SIZE 4088
-#    else
-#     ifdef JUMBO_FRAME_SIZE_6K
-#     define JUMBO_FRAME_SIZE 6144
-#     define __IPG_RXFRAG_SIZE 4088
-#     else
-#      ifdef JUMBO_FRAME_SIZE_7K
-#      define JUMBO_FRAME_SIZE 7168
-#      define __IPG_RXFRAG_SIZE 4088
-#      else
-#       ifdef JUMBO_FRAME_SIZE_8K
-#       define JUMBO_FRAME_SIZE 8192
-#       define __IPG_RXFRAG_SIZE 4088
-#       else
-#        ifdef JUMBO_FRAME_SIZE_9K
-#        define JUMBO_FRAME_SIZE 9216
-#        define __IPG_RXFRAG_SIZE 4088
-#        else
-#         ifdef JUMBO_FRAME_SIZE_10K
-#         define JUMBO_FRAME_SIZE 10240
-#         define __IPG_RXFRAG_SIZE 4088
-#         else
-#         define JUMBO_FRAME_SIZE 4096
-#         endif
-#        endif
-#       endif
-#      endif
-#     endif
-#    endif
-#   endif
-#  endif
-# endif
-#endif
-
-/* Size of allocated received buffers. Nominally 0x0600.
- * Define larger if expecting jumbo frames.
- */
-#ifdef JUMBO_FRAME
-/* IPG_TXFRAG_SIZE must <= 0x2b00, or TX will crash */
-#define		IPG_TXFRAG_SIZE		JUMBO_FRAME_SIZE
-#else
-#define		IPG_TXFRAG_SIZE		0 /* use default MTU */
-#endif
-
-/* Size of allocated received buffers. Nominally 0x0600.
- * Define larger if expecting jumbo frames.
- */
-#ifdef JUMBO_FRAME
-/* 4088 = 4096 - 8 */
-#define		IPG_RXFRAG_SIZE		__IPG_RXFRAG_SIZE
-#define     IPG_RXSUPPORT_SIZE   IPG_MAX_RXFRAME_SIZE
-#else
-#define		IPG_RXFRAG_SIZE		0x0600
-#define     IPG_RXSUPPORT_SIZE   IPG_RXFRAG_SIZE
-#endif
-
-/* IPG_MAX_RXFRAME_SIZE <= IPG_RXFRAG_SIZE */
-#ifdef JUMBO_FRAME
-#define		IPG_MAX_RXFRAME_SIZE		JUMBO_FRAME_SIZE
-#else
-#define		IPG_MAX_RXFRAME_SIZE		0x0600
-#endif
-
 #define		IPG_RFDLIST_LENGTH		0x100
 
 /* Maximum number of RFDs to process per interrupt.

Re: [RFC/PATCH] ipg: add jumbo frame support kconfig option

[Andrew Savchenko]

[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]

Hi Pekka,

On Friday 20 June 2008 15:12, Pekka Enberg wrote:
> Hi Andrew,
>
> (Please don't trim the cc.)

Oh, my apologies. I'm not subscribed to the list, so I mailed only 
to emails found on lkml web page with original top mail.

> On Fri, Jun 20, 2008 at 9:37 AM, Andrew Savchenko 
> <Bircoph@list.ru> wrote: 
[...]
> As I don't have the hardware, I would appreciate if you could
> test the following quilt tree:
>
>  
> http://www.kernel.org/pub/linux/kernel/people/penberg/patches/ip
>g/2.6.26-rc6/
>
> [Note: in case you're not familiar with quilt, the patches need
> to be applied in the order specified by the quilt 'series' file
> included there.]

Thanks for the advice. I installed quilt on my system and applied 
your patchset to 2.6.26-rc6 kernel tree.

> You can enable 10KB jumbo frames either by passing:
>
>   ipg.jumbo=10k
>
> to the kernel command line or, alternatively, load the ipg
> module as follows:
>
>   modprobe ipg jumbo=10k
>
> You cannot enable or disable jumbo frame support once the module
> is loaded so if you want to try out different frame sizes, you
> first need to unload the module.

Ok, but in practice I need to reboot the system: 1) usually I 
compile into the kernel all code related to frequently used 
hardware. 2) This is a remote system, so reload of network module 
which is in use will be quite tricky.

I tested it with both jumbo frames disabled (no options), and 
enabled for 10k frames: ipg.jumbo=10k. It seems to work fine with 
all acceptable MTUs (max MTU == 10240).

Sincerely yours,
Andrew

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC/PATCH] ipg: add jumbo frame support kconfig option

[Stephen Hemminger]

On Fri, 20 Jun 2008 14:12:51 +0300
"Pekka Enberg" <penberg@cs.helsinki.fi> wrote:

> Hi Andrew,
> 
> (Please don't trim the cc.)
> 
> On Fri, Jun 20, 2008 at 9:37 AM, Andrew Savchenko <Bircoph@list.ru> wrote:
> > Hello,
> >
> > On Wed, 27 Feb 2008 09:44:44 +0200 (EET), Pekka J Enberg wrote:
> >> Stephen Hemminger wrote:
> >> > > Please don't do this as a config option. Config options seem
> >> > > like a good idea to developers but are difficult for
> >> > > distributions.  Either support jumbo frames by default or
> >> > > not at all.
> >>
> >> On Tue, 26 Feb 2008, Jeff Garzik wrote:
> >> > Agreed, but to be more specific... always compile in jumbo
> >> > frame support. You don't have to enable it at runtime by
> >> > default, though.
> >>
> >> Ok, so a sysfs attribute would be sufficient, I suppose? I don't
> >> have the hardware though, so I don't even know if the jumbo
> >> frame code actually works at this point hence the RFC. So I'll
> >> wait for someone to test this before sending a new patch. Thanks
> >> for the review!
> >
> > I own such card:
> > 02:02.0 Ethernet controller: Sundance Technology Inc / IC Plus Corp
> > IP1000 Family Gigabit Ethernet (rev 41)
> > and I use patch similar to yours in order to enable Jumbo Frames.
> > See attachment for details, patch is done for linux-2.6.25.6.
> >
> > It works fine for me. However, I enabled 10K jumbo boxes instead of
> > default 4K value, activated using your patch. Reason is simple: I
> > need at least 9000-byte jumbo boxes in my network.
> >
> > However, I have one request for you: please, make in possible to
> > choose max MTU via kernel config option. In different environments
> > different people may need different max MTU sizes (2K...10K).
> > Recommended value is 4K (as described in the original driver from
> > manufacturer, which was remake into kernel's one), but, for
> > example, I'm interested in larger MTU values. OTOH larger MTU
> > values require more memory and so on. For me, as for end user of
> > Linux kernel, a possibility to select desired max MTU is very
> > attractive and is definitely in no doubt more convenient than
> > patching the kernel myself each time.
> >
> > So, feel free to add jumbo boxes on ipg support to the mainstream
> > and, please, make in possible to select max MTU; 4K as default
> > will be fine.
> 
> As I don't have the hardware, I would appreciate if you could test
> the following quilt tree:
> 
>   http://www.kernel.org/pub/linux/kernel/people/penberg/patches/ipg/2.6.26-rc6/
> 
> [Note: in case you're not familiar with quilt, the patches need to
>  be applied in the order specified by the quilt 'series' file
>  included there.]
> 
> You can enable 10KB jumbo frames either by passing:
> 
>   ipg.jumbo=10k
> 
> to the kernel command line or, alternatively, load the ipg module
> as follows:
> 
>   modprobe ipg jumbo=10k
> 
> You cannot enable or disable jumbo frame support once the module
> is loaded so if you want to try out different frame sizes, you
> first need to unload the module.
> 
>                            Pekka

The code should enable/disable anything it needs based on the MTU size
in the change_mtu call. If the driver needs to reallocate buffers, change receiver
settings, make sacrifices to deities, or phone home that is the place to do it.
In other words no compile or config option fro MTU settings please.

Re: [RFC/PATCH] ipg: add jumbo frame support kconfig option

[Pekka Enberg]

Hi Andrew,

(Please don't trim the cc.)

On Fri, Jun 20, 2008 at 9:37 AM, Andrew Savchenko <Bircoph@list.ru> wrote:
> Hello,
>
> On Wed, 27 Feb 2008 09:44:44 +0200 (EET), Pekka J Enberg wrote:
>> Stephen Hemminger wrote:
>> > > Please don't do this as a config option. Config options seem
>> > > like a good idea to developers but are difficult for
>> > > distributions.  Either support jumbo frames by default or
>> > > not at all.
>>
>> On Tue, 26 Feb 2008, Jeff Garzik wrote:
>> > Agreed, but to be more specific... always compile in jumbo
>> > frame support. You don't have to enable it at runtime by
>> > default, though.
>>
>> Ok, so a sysfs attribute would be sufficient, I suppose? I don't
>> have the hardware though, so I don't even know if the jumbo
>> frame code actually works at this point hence the RFC. So I'll
>> wait for someone to test this before sending a new patch. Thanks
>> for the review!
>
> I own such card:
> 02:02.0 Ethernet controller: Sundance Technology Inc / IC Plus Corp
> IP1000 Family Gigabit Ethernet (rev 41)
> and I use patch similar to yours in order to enable Jumbo Frames.
> See attachment for details, patch is done for linux-2.6.25.6.
>
> It works fine for me. However, I enabled 10K jumbo boxes instead of
> default 4K value, activated using your patch. Reason is simple: I
> need at least 9000-byte jumbo boxes in my network.
>
> However, I have one request for you: please, make in possible to
> choose max MTU via kernel config option. In different environments
> different people may need different max MTU sizes (2K...10K).
> Recommended value is 4K (as described in the original driver from
> manufacturer, which was remake into kernel's one), but, for
> example, I'm interested in larger MTU values. OTOH larger MTU
> values require more memory and so on. For me, as for end user of
> Linux kernel, a possibility to select desired max MTU is very
> attractive and is definitely in no doubt more convenient than
> patching the kernel myself each time.
>
> So, feel free to add jumbo boxes on ipg support to the mainstream
> and, please, make in possible to select max MTU; 4K as default
> will be fine.

As I don't have the hardware, I would appreciate if you could test
the following quilt tree:

  http://www.kernel.org/pub/linux/kernel/people/penberg/patches/ipg/2.6.26-rc6/

[Note: in case you're not familiar with quilt, the patches need to
 be applied in the order specified by the quilt 'series' file
 included there.]

You can enable 10KB jumbo frames either by passing:

  ipg.jumbo=10k

to the kernel command line or, alternatively, load the ipg module
as follows:

  modprobe ipg jumbo=10k

You cannot enable or disable jumbo frame support once the module
is loaded so if you want to try out different frame sizes, you
first need to unload the module.

                           Pekka

Re: [RFC/PATCH] ipg: add jumbo frame support kconfig option

[Andrew Savchenko]

[-- Attachment #1.1: Type: text/plain, Size: 2058 bytes --]

Hello,

On Wed, 27 Feb 2008 09:44:44 +0200 (EET), Pekka J Enberg wrote:
> Stephen Hemminger wrote:
> > > Please don't do this as a config option. Config options seem
> > > like a good idea to developers but are difficult for
> > > distributions.  Either support jumbo frames by default or
> > > not at all.
>
> On Tue, 26 Feb 2008, Jeff Garzik wrote:
> > Agreed, but to be more specific... always compile in jumbo
> > frame support. You don't have to enable it at runtime by
> > default, though.
>
> Ok, so a sysfs attribute would be sufficient, I suppose? I don't
> have the hardware though, so I don't even know if the jumbo
> frame code actually works at this point hence the RFC. So I'll
> wait for someone to test this before sending a new patch. Thanks
> for the review!

I own such card:
02:02.0 Ethernet controller: Sundance Technology Inc / IC Plus Corp 
IP1000 Family Gigabit Ethernet (rev 41)
and I use patch similar to yours in order to enable Jumbo Frames. 
See attachment for details, patch is done for linux-2.6.25.6.

It works fine for me. However, I enabled 10K jumbo boxes instead of 
default 4K value, activated using your patch. Reason is simple: I 
need at least 9000-byte jumbo boxes in my network.

However, I have one request for you: please, make in possible to 
choose max MTU via kernel config option. In different environments 
different people may need different max MTU sizes (2K...10K). 
Recommended value is 4K (as described in the original driver from 
manufacturer, which was remake into kernel's one), but, for 
example, I'm interested in larger MTU values. OTOH larger MTU 
values require more memory and so on. For me, as for end user of 
Linux kernel, a possibility to select desired max MTU is very 
attractive and is definitely in no doubt more convenient than 
patching the kernel myself each time.

So, feel free to add jumbo boxes on ipg support to the mainstream 
and, please, make in possible to select max MTU; 4K as default 
will be fine.

Sincerely yours,
Andrew

[-- Attachment #1.2: ipg-jumbo.patch --]
[-- Type: text/plain, Size: 1239 bytes --]

--- drivers/net/ipg.c.orig	2008-04-17 06:49:44.000000000 +0400
+++ drivers/net/ipg.c	2008-06-18 04:46:06.000000000 +0400
@@ -42,7 +42,8 @@
 #define ipg_r16(reg)		ioread16(ioaddr + (reg))
 #define ipg_r8(reg)		ioread8(ioaddr + (reg))
 
-#define JUMBO_FRAME_4k_ONLY
+#define JUMBO_FRAME
+#define JUMBO_FRAME_SIZE_10K
 enum {
 	netdev_io_size = 128
 };
@@ -1267,7 +1268,7 @@
 			return;
 
 		if (jumbo->found_start) {
-			int framelen, endframelen;
+			int framelen, endframeLen;
 
 			framelen = le64_to_cpu(rxfd->rfs) & IPG_RFS_RXFRAMELEN;
 
@@ -1352,16 +1353,16 @@
 
 		switch (ipg_nic_rx_check_frame_type(dev)) {
 		case FRAME_WITH_START_WITH_END:
-			ipg_nic_rx_with_start_and_end(dev, tp, rxfd, entry);
+			ipg_nic_rx_with_start_and_end(dev, sp, rxfd, entry);
 			break;
 		case FRAME_WITH_START:
-			ipg_nic_rx_with_start(dev, tp, rxfd, entry);
+			ipg_nic_rx_with_start(dev, sp, rxfd, entry);
 			break;
 		case FRAME_WITH_END:
-			ipg_nic_rx_with_end(dev, tp, rxfd, entry);
+			ipg_nic_rx_with_end(dev, sp, rxfd, entry);
 			break;
 		case FRAME_NO_START_NO_END:
-			ipg_nic_rx_no_start_no_end(dev, tp, rxfd, entry);
+			ipg_nic_rx_no_start_no_end(dev, sp, rxfd, entry);
 			break;
 		}
 	}

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[RFC/PATCH] ipg: add jumbo frame support kconfig option

[Pekka J Enberg]

From: Pekka Enberg <penberg@cs.helsinki.fi>

Convert the internal JUMBO_FRAME #ifdef to CONFIG_IP1000_JUMBO_FRAME proper and
fix compilation errors.

Cc: Francois Romieu <romieu@fr.zoreil.com>
Cc: Sorbica Shieh <sorbica@icplus.com.tw>
Cc: Jesse Huang <jesse@icplus.com.tw>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---
 drivers/net/Kconfig |    8 ++++++++
 drivers/net/ipg.c   |   21 ++++++++++-----------
 drivers/net/ipg.h   |   11 ++++++-----
 3 files changed, 24 insertions(+), 16 deletions(-)

Index: linux-2.6/drivers/net/Kconfig
===================================================================
--- linux-2.6.orig/drivers/net/Kconfig
+++ linux-2.6/drivers/net/Kconfig
@@ -2029,6 +2029,14 @@ config IP1000
 	  To compile this driver as a module, choose M here: the module
 	  will be called ipg.  This is recommended.
 
+config IP1000_JUMBO_FRAME
+	bool "Support for jumbo frames (EXPERIMENTAL)"
+	depends on IP1000 && EXPERIMENTAL
+	help
+	  This option enables jumbo frame support for the IP1000 driver.
+
+	  If in doubt, say N.
+
 config IGB
        tristate "Intel(R) 82575 PCI-Express Gigabit Ethernet support"
        depends on PCI
Index: linux-2.6/drivers/net/ipg.c
===================================================================
--- linux-2.6.orig/drivers/net/ipg.c
+++ linux-2.6/drivers/net/ipg.c
@@ -42,7 +42,6 @@
 #define ipg_r16(reg)		ioread16(ioaddr + (reg))
 #define ipg_r8(reg)		ioread8(ioaddr + (reg))
 
-#define JUMBO_FRAME_4k_ONLY
 enum {
 	netdev_io_size = 128
 };
@@ -1079,7 +1078,7 @@ static int ipg_nic_rxrestore(struct net_
 	return 0;
 }
 
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 
 /* use jumboindex and jumbosize to control jumbo frame status
  * initial status is jumboindex=-1 and jumbosize=0
@@ -1274,7 +1273,7 @@ static void ipg_nic_rx_with_end(struct n
 
 			framelen = le64_to_cpu(rxfd->rfs) & IPG_RFS_RXFRAMELEN;
 
-			endframeLen = framelen - jumbo->current_size;
+			endframelen = framelen - jumbo->current_size;
 			/*
 			if (framelen > IPG_RXFRAG_SIZE)
 				framelen=IPG_RXFRAG_SIZE;
@@ -1282,8 +1281,8 @@ static void ipg_nic_rx_with_end(struct n
 			if (framelen > IPG_RXSUPPORT_SIZE)
 				dev_kfree_skb_irq(jumbo->skb);
 			else {
-				memcpy(skb_put(jumbo->skb, endframeLen),
-				       skb->data, endframeLen);
+				memcpy(skb_put(jumbo->skb, endframelen),
+				       skb->data, endframelen);
 
 				jumbo->skb->protocol =
 				    eth_type_trans(jumbo->skb, dev);
@@ -1355,16 +1354,16 @@ static int ipg_nic_rx(struct net_device 
 
 		switch (ipg_nic_rx_check_frame_type(dev)) {
 		case FRAME_WITH_START_WITH_END:
-			ipg_nic_rx_with_start_and_end(dev, tp, rxfd, entry);
+			ipg_nic_rx_with_start_and_end(dev, sp, rxfd, entry);
 			break;
 		case FRAME_WITH_START:
-			ipg_nic_rx_with_start(dev, tp, rxfd, entry);
+			ipg_nic_rx_with_start(dev, sp, rxfd, entry);
 			break;
 		case FRAME_WITH_END:
-			ipg_nic_rx_with_end(dev, tp, rxfd, entry);
+			ipg_nic_rx_with_end(dev, sp, rxfd, entry);
 			break;
 		case FRAME_NO_START_NO_END:
-			ipg_nic_rx_no_start_no_end(dev, tp, rxfd, entry);
+			ipg_nic_rx_no_start_no_end(dev, sp, rxfd, entry);
 			break;
 		}
 	}
@@ -1595,7 +1594,7 @@ static irqreturn_t ipg_interrupt_handler
 
 	IPG_DEBUG_MSG("_interrupt_handler\n");
 
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 	ipg_nic_rxrestore(dev);
 #endif
 	spin_lock(&sp->lock);
@@ -1807,7 +1806,7 @@ static int ipg_nic_open(struct net_devic
 	if (ipg_config_autoneg(dev) < 0)
 		printk(KERN_INFO "%s: Auto-negotiation error.\n", dev->name);
 
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 	/* initialize JUMBO Frame control variable */
 	sp->jumbo.found_start = 0;
 	sp->jumbo.current_size = 0;
Index: linux-2.6/drivers/net/ipg.h
===================================================================
--- linux-2.6.orig/drivers/net/ipg.h
+++ linux-2.6/drivers/net/ipg.h
@@ -536,7 +536,7 @@ enum ipg_regs {
  */
 #define		IPG_FRAMESBETWEENTXDMACOMPLETES 0x1
 
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 
 # ifdef JUMBO_FRAME_SIZE_2K
 # define JUMBO_FRAME_SIZE 2048
@@ -575,6 +575,7 @@ enum ipg_regs {
 #         define __IPG_RXFRAG_SIZE 4088
 #         else
 #         define JUMBO_FRAME_SIZE 4096
+#         define __IPG_RXFRAG_SIZE 4088
 #         endif
 #        endif
 #       endif
@@ -589,7 +590,7 @@ enum ipg_regs {
 /* Size of allocated received buffers. Nominally 0x0600.
  * Define larger if expecting jumbo frames.
  */
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 /* IPG_TXFRAG_SIZE must <= 0x2b00, or TX will crash */
 #define		IPG_TXFRAG_SIZE		JUMBO_FRAME_SIZE
 #endif
@@ -597,7 +598,7 @@ enum ipg_regs {
 /* Size of allocated received buffers. Nominally 0x0600.
  * Define larger if expecting jumbo frames.
  */
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 /* 4088 = 4096 - 8 */
 #define		IPG_RXFRAG_SIZE		__IPG_RXFRAG_SIZE
 #define     IPG_RXSUPPORT_SIZE   IPG_MAX_RXFRAME_SIZE
@@ -607,7 +608,7 @@ enum ipg_regs {
 #endif
 
 /* IPG_MAX_RXFRAME_SIZE <= IPG_RXFRAG_SIZE */
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 #define		IPG_MAX_RXFRAME_SIZE		JUMBO_FRAME_SIZE
 #else
 #define		IPG_MAX_RXFRAME_SIZE		0x0600
@@ -786,7 +787,7 @@ struct ipg_nic_private {
 	unsigned int tx_dirty;
 	unsigned int rx_current;
 	unsigned int rx_dirty;
-#ifdef JUMBO_FRAME
+#ifdef CONFIG_IP1000_JUMBO_FRAME
 	struct ipg_jumbo jumbo;
 #endif
 	unsigned int rx_buf_sz;