[PATCH] set mtu from bridge also on vif interface

[PATCH] set mtu from bridge also on vif interface

[Olaf Hering]

Apply mtu size from bridge interface also in vif interface.
This depends on a kernel change which allows arbitrary mtu sizes until
the frontend driver has connected to the backend driver. Without this
kernel change, the vif mtu size will be limited to 1500 even with this
change to the vif-bridge script.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 tools/hotplug/Linux/vif-bridge |    5 +++++
 1 file changed, 5 insertions(+)

--- xen-unstable.hg-4.1.22870.orig/tools/hotplug/Linux/vif-bridge
+++ xen-unstable.hg-4.1.22870/tools/hotplug/Linux/vif-bridge
@@ -82,6 +82,11 @@ fi
 case "$command" in
     online)
         setup_virtual_bridge_port "$dev"
+        mtu="`ip link show $bridge | awk '/mtu/ { print $5 }'`"
+        if [ -n "$mtu" ] && [ "$mtu" -gt 0 ]
+        then
+                ip link set $dev mtu $mtu || :
+        fi
         add_to_bridge "$bridge" "$dev"
         ;;

[PATCH] netback: allow arbitrary mtu size until frontend connects

[Olaf Hering]

Allow arbitrary mtu size until the frontend is connected.  Once the
connection is established, adjust mtu by checking if the backend
supports the 'feature-sg'.  If the backend does support it, keep the
current mtu. Otherwise set it to the default value, which is 1500.

This helps the vif-bridge hotplug script to set the mtu size to 9000
while bringing up the guest.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 drivers/xen/netback/common.h    |    8 ++++++++
 drivers/xen/netback/interface.c |    8 +++++++-
 drivers/xen/netback/xenbus.c    |    9 +++++++++
 3 files changed, 24 insertions(+), 1 deletion(-)

--- linux-2.6.18-xen.hg.orig/drivers/xen/netback/common.h
+++ linux-2.6.18-xen.hg/drivers/xen/netback/common.h
@@ -82,6 +82,8 @@ typedef struct netif_st {
 	u8 can_queue:1;	/* can queue packets for receiver? */
 	u8 copying_receiver:1;	/* copy packets to receiver?       */
 
+	u8 frontend_connected:1; /* is frontend already connected? */
+
 	/* Allow netif_be_start_xmit() to peek ahead in the rx request ring. */
 	RING_IDX rx_req_cons_peek;
 
@@ -217,4 +219,10 @@ static inline int netbk_can_sg(struct ne
 	return netif->features & NETIF_F_SG;
 }
 
+static inline int netbk_frontend_connected(struct net_device *dev)
+{
+	netif_t *netif = netdev_priv(dev);
+	return !!netif->frontend_connected;
+}
+
 #endif /* __NETIF__BACKEND__COMMON_H__ */
--- linux-2.6.18-xen.hg.orig/drivers/xen/netback/interface.c
+++ linux-2.6.18-xen.hg/drivers/xen/netback/interface.c
@@ -83,9 +83,13 @@ static int net_close(struct net_device *
 	return 0;
 }
 
+/* accept the max value if frontend is not yet connected */
 static int netbk_change_mtu(struct net_device *dev, int mtu)
 {
-	int max = netbk_can_sg(dev) ? 65535 - ETH_HLEN : ETH_DATA_LEN;
+	int max = 65535 - ETH_HLEN;
+
+	if (netbk_frontend_connected(dev))
+		max = netbk_can_sg(dev) ? max : ETH_DATA_LEN;
 
 	if (mtu > max)
 		return -EINVAL;
@@ -359,6 +363,8 @@ err_rx:
 
 void netif_disconnect(netif_t *netif)
 {
+	netif->frontend_connected = 0;
+
 	if (netback_carrier_ok(netif)) {
 		rtnl_lock();
 		netback_carrier_off(netif);
--- linux-2.6.18-xen.hg.orig/drivers/xen/netback/xenbus.c
+++ linux-2.6.18-xen.hg/drivers/xen/netback/xenbus.c
@@ -329,6 +329,13 @@ static int xen_net_read_mac(struct xenbu
 	return 0;
 }
 
+static void xen_net_adjust_mtu(netif_t *netif)
+{
+	if (!netif->dev->change_mtu)
+		return;
+	netif->dev->change_mtu(netif->dev, netif->dev->mtu);
+}
+
 static void connect(struct backend_info *be)
 {
 	int err;
@@ -338,6 +345,8 @@ static void connect(struct backend_info
 	if (err)
 		return;
 
+	be->netif->frontend_connected = 1;
+	xen_net_adjust_mtu(be->netif);
 	err = xen_net_read_mac(dev, be->netif->fe_dev_addr);
 	if (err) {
 		xenbus_dev_fatal(dev, err, "parsing %s/mac", dev->nodename);

Re: [PATCH] netback: allow arbitrary mtu size until frontend connects

[Jan Beulich]

>>> On 06.02.11 at 14:42, Olaf Hering <olaf@aepfle.de> wrote:
> Allow arbitrary mtu size until the frontend is connected.  Once the
> connection is established, adjust mtu by checking if the backend
> supports the 'feature-sg'.  If the backend does support it, keep the
> current mtu. Otherwise set it to the default value, which is 1500.
> 
> This helps the vif-bridge hotplug script to set the mtu size to 9000
> while bringing up the guest.

Isn't this functionally the same as
http://xenbits.xen.org/XCP/linux-2.6.32.pq.hg?file/043b76e4943c/xen-netback-Allow-setting-of-large-MTU-before-rings.patch
or its pv-ops parent https://git.kernel.org/?p=linux/kernel/git/jeremy/xen.git;a=commitdiff;h=bee2eec2355c4bf4e149a426d5e30527162de566
(in which case it would seem preferable to pull in that change)?

Ian, looking at that patch of yours again, the adjustment to
netbk_set_sg() seems a little odd: Why is it necessary to
reduce dev->mtu unconditionally (i.e. independent of the
value of "data") here?

Jan

> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> ---
>  drivers/xen/netback/common.h    |    8 ++++++++
>  drivers/xen/netback/interface.c |    8 +++++++-
>  drivers/xen/netback/xenbus.c    |    9 +++++++++
>  3 files changed, 24 insertions(+), 1 deletion(-)
> 
> --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/common.h
> +++ linux-2.6.18-xen.hg/drivers/xen/netback/common.h
> @@ -82,6 +82,8 @@ typedef struct netif_st {
>  	u8 can_queue:1;	/* can queue packets for receiver? */
>  	u8 copying_receiver:1;	/* copy packets to receiver?       */
>  
> +	u8 frontend_connected:1; /* is frontend already connected? */
> +
>  	/* Allow netif_be_start_xmit() to peek ahead in the rx request ring. */
>  	RING_IDX rx_req_cons_peek;
>  
> @@ -217,4 +219,10 @@ static inline int netbk_can_sg(struct ne
>  	return netif->features & NETIF_F_SG;
>  }
>  
> +static inline int netbk_frontend_connected(struct net_device *dev)
> +{
> +	netif_t *netif = netdev_priv(dev);
> +	return !!netif->frontend_connected;
> +}
> +
>  #endif /* __NETIF__BACKEND__COMMON_H__ */
> --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/interface.c
> +++ linux-2.6.18-xen.hg/drivers/xen/netback/interface.c
> @@ -83,9 +83,13 @@ static int net_close(struct net_device *
>  	return 0;
>  }
>  
> +/* accept the max value if frontend is not yet connected */
>  static int netbk_change_mtu(struct net_device *dev, int mtu)
>  {
> -	int max = netbk_can_sg(dev) ? 65535 - ETH_HLEN : ETH_DATA_LEN;
> +	int max = 65535 - ETH_HLEN;
> +
> +	if (netbk_frontend_connected(dev))
> +		max = netbk_can_sg(dev) ? max : ETH_DATA_LEN;
>  
>  	if (mtu > max)
>  		return -EINVAL;
> @@ -359,6 +363,8 @@ err_rx:
>  
>  void netif_disconnect(netif_t *netif)
>  {
> +	netif->frontend_connected = 0;
> +
>  	if (netback_carrier_ok(netif)) {
>  		rtnl_lock();
>  		netback_carrier_off(netif);
> --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/xenbus.c
> +++ linux-2.6.18-xen.hg/drivers/xen/netback/xenbus.c
> @@ -329,6 +329,13 @@ static int xen_net_read_mac(struct xenbu
>  	return 0;
>  }
>  
> +static void xen_net_adjust_mtu(netif_t *netif)
> +{
> +	if (!netif->dev->change_mtu)
> +		return;
> +	netif->dev->change_mtu(netif->dev, netif->dev->mtu);
> +}
> +
>  static void connect(struct backend_info *be)
>  {
>  	int err;
> @@ -338,6 +345,8 @@ static void connect(struct backend_info
>  	if (err)
>  		return;
>  
> +	be->netif->frontend_connected = 1;
> +	xen_net_adjust_mtu(be->netif);
>  	err = xen_net_read_mac(dev, be->netif->fe_dev_addr);
>  	if (err) {
>  		xenbus_dev_fatal(dev, err, "parsing %s/mac", dev->nodename);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com 
> http://lists.xensource.com/xen-devel 

Re: [PATCH] set mtu from bridge also on vif interface

[Ian Campbell]

On Sun, 2011-02-06 at 13:41 +0000, Olaf Hering wrote:
> Apply mtu size from bridge interface also in vif interface.
> This depends on a kernel change which allows arbitrary mtu sizes until
> the frontend driver has connected to the backend driver. Without this
> kernel change, the vif mtu size will be limited to 1500 even with this
> change to the vif-bridge script.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

In my experience the bridge will only allow it's MTU to be set to the
minimum of all the ports (or 1500ish if there are no ports) and will
reset to the new minimum when you add a new port.

So does this patch do anything in the absence of the ability to
configure the MTU of a VIF from the toolstack?

Ian.

> 
> ---
>  tools/hotplug/Linux/vif-bridge |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> --- xen-unstable.hg-4.1.22870.orig/tools/hotplug/Linux/vif-bridge
> +++ xen-unstable.hg-4.1.22870/tools/hotplug/Linux/vif-bridge
> @@ -82,6 +82,11 @@ fi
>  case "$command" in
>      online)
>          setup_virtual_bridge_port "$dev"
> +        mtu="`ip link show $bridge | awk '/mtu/ { print $5 }'`"
> +        if [ -n "$mtu" ] && [ "$mtu" -gt 0 ]
> +        then
> +                ip link set $dev mtu $mtu || :
> +        fi
>          add_to_bridge "$bridge" "$dev"
>          ;;
>  
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] netback: allow arbitrary mtu size until frontend connects

[Ian Campbell]

On Mon, 2011-02-07 at 09:03 +0000, Jan Beulich wrote:
> >>> On 06.02.11 at 14:42, Olaf Hering <olaf@aepfle.de> wrote:
> > Allow arbitrary mtu size until the frontend is connected.  Once the
> > connection is established, adjust mtu by checking if the backend
> > supports the 'feature-sg'.  If the backend does support it, keep the
> > current mtu. Otherwise set it to the default value, which is 1500.
> > 
> > This helps the vif-bridge hotplug script to set the mtu size to 9000
> > while bringing up the guest.
> 
> Isn't this functionally the same as
> http://xenbits.xen.org/XCP/linux-2.6.32.pq.hg?file/043b76e4943c/xen-netback-Allow-setting-of-large-MTU-before-rings.patch
> or its pv-ops parent https://git.kernel.org/?p=linux/kernel/git/jeremy/xen.git;a=commitdiff;h=bee2eec2355c4bf4e149a426d5e30527162de566
> (in which case it would seem preferable to pull in that change)?
> 
> Ian, looking at that patch of yours again, the adjustment to
> netbk_set_sg() seems a little odd: Why is it necessary to
> reduce dev->mtu unconditionally (i.e. independent of the
> value of "data") here?

Hmm. I don't recall. Seems like a bug looking at it now...

Luckily for me it appears to have been correct by Paul in
d532fa93d4eeabbfc0176a6a9a93b0d6ade3f6c4

Ian.

> 
> Jan
> 
> > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > 
> > ---
> >  drivers/xen/netback/common.h    |    8 ++++++++
> >  drivers/xen/netback/interface.c |    8 +++++++-
> >  drivers/xen/netback/xenbus.c    |    9 +++++++++
> >  3 files changed, 24 insertions(+), 1 deletion(-)
> > 
> > --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/common.h
> > +++ linux-2.6.18-xen.hg/drivers/xen/netback/common.h
> > @@ -82,6 +82,8 @@ typedef struct netif_st {
> >  	u8 can_queue:1;	/* can queue packets for receiver? */
> >  	u8 copying_receiver:1;	/* copy packets to receiver?       */
> >  
> > +	u8 frontend_connected:1; /* is frontend already connected? */
> > +
> >  	/* Allow netif_be_start_xmit() to peek ahead in the rx request ring. */
> >  	RING_IDX rx_req_cons_peek;
> >  
> > @@ -217,4 +219,10 @@ static inline int netbk_can_sg(struct ne
> >  	return netif->features & NETIF_F_SG;
> >  }
> >  
> > +static inline int netbk_frontend_connected(struct net_device *dev)
> > +{
> > +	netif_t *netif = netdev_priv(dev);
> > +	return !!netif->frontend_connected;
> > +}
> > +
> >  #endif /* __NETIF__BACKEND__COMMON_H__ */
> > --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/interface.c
> > +++ linux-2.6.18-xen.hg/drivers/xen/netback/interface.c
> > @@ -83,9 +83,13 @@ static int net_close(struct net_device *
> >  	return 0;
> >  }
> >  
> > +/* accept the max value if frontend is not yet connected */
> >  static int netbk_change_mtu(struct net_device *dev, int mtu)
> >  {
> > -	int max = netbk_can_sg(dev) ? 65535 - ETH_HLEN : ETH_DATA_LEN;
> > +	int max = 65535 - ETH_HLEN;
> > +
> > +	if (netbk_frontend_connected(dev))
> > +		max = netbk_can_sg(dev) ? max : ETH_DATA_LEN;
> >  
> >  	if (mtu > max)
> >  		return -EINVAL;
> > @@ -359,6 +363,8 @@ err_rx:
> >  
> >  void netif_disconnect(netif_t *netif)
> >  {
> > +	netif->frontend_connected = 0;
> > +
> >  	if (netback_carrier_ok(netif)) {
> >  		rtnl_lock();
> >  		netback_carrier_off(netif);
> > --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/xenbus.c
> > +++ linux-2.6.18-xen.hg/drivers/xen/netback/xenbus.c
> > @@ -329,6 +329,13 @@ static int xen_net_read_mac(struct xenbu
> >  	return 0;
> >  }
> >  
> > +static void xen_net_adjust_mtu(netif_t *netif)
> > +{
> > +	if (!netif->dev->change_mtu)
> > +		return;
> > +	netif->dev->change_mtu(netif->dev, netif->dev->mtu);
> > +}
> > +
> >  static void connect(struct backend_info *be)
> >  {
> >  	int err;
> > @@ -338,6 +345,8 @@ static void connect(struct backend_info
> >  	if (err)
> >  		return;
> >  
> > +	be->netif->frontend_connected = 1;
> > +	xen_net_adjust_mtu(be->netif);
> >  	err = xen_net_read_mac(dev, be->netif->fe_dev_addr);
> >  	if (err) {
> >  		xenbus_dev_fatal(dev, err, "parsing %s/mac", dev->nodename);
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com 
> > http://lists.xensource.com/xen-devel 
> 
> 
> 

Re: [PATCH] set mtu from bridge also on vif interface

[Olaf Hering]

On Mon, Feb 07, Ian Campbell wrote:

> On Sun, 2011-02-06 at 13:41 +0000, Olaf Hering wrote:
> > Apply mtu size from bridge interface also in vif interface.
> > This depends on a kernel change which allows arbitrary mtu sizes until
> > the frontend driver has connected to the backend driver. Without this
> > kernel change, the vif mtu size will be limited to 1500 even with this
> > change to the vif-bridge script.
> > 
> > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> In my experience the bridge will only allow it's MTU to be set to the
> minimum of all the ports (or 1500ish if there are no ports) and will
> reset to the new minimum when you add a new port.
> 
> So does this patch do anything in the absence of the ability to
> configure the MTU of a VIF from the toolstack?

This depends if the bridge interface is part of the Xen toolstack
configuration or if the bridge is part of the system configuration. In
my setup the bridge is part of the system configuration and gets its mtu
from the config of the physical interface, eth0 in my case. So br0 has
mtu of 9000 already when vif-bridge runs.

With this change, the vif interfaces follow the system configuration.

Olaf

Re: [PATCH] netback: allow arbitrary mtu size until frontend connects

[Olaf Hering]

On Mon, Feb 07, Jan Beulich wrote:

> >>> On 06.02.11 at 14:42, Olaf Hering <olaf@aepfle.de> wrote:
> > Allow arbitrary mtu size until the frontend is connected.  Once the
> > connection is established, adjust mtu by checking if the backend
> > supports the 'feature-sg'.  If the backend does support it, keep the
> > current mtu. Otherwise set it to the default value, which is 1500.
> > 
> > This helps the vif-bridge hotplug script to set the mtu size to 9000
> > while bringing up the guest.
> 
> Isn't this functionally the same as
> http://xenbits.xen.org/XCP/linux-2.6.32.pq.hg?file/043b76e4943c/xen-netback-Allow-setting-of-large-MTU-before-rings.patch
> or its pv-ops parent https://git.kernel.org/?p=linux/kernel/git/jeremy/xen.git;a=commitdiff;h=bee2eec2355c4bf4e149a426d5e30527162de566
> (in which case it would seem preferable to pull in that change)?

Either approach is ok.

Could one of the changes be backported and applied to the 2.6.18.hg tree?

Olaf

Re: [PATCH] netback: allow arbitrary mtu size until frontend connects

[Ian Campbell]

On Mon, 2011-02-07 at 09:26 +0000, Olaf Hering wrote:
> On Mon, Feb 07, Jan Beulich wrote:
> 
> > >>> On 06.02.11 at 14:42, Olaf Hering <olaf@aepfle.de> wrote:
> > > Allow arbitrary mtu size until the frontend is connected.  Once the
> > > connection is established, adjust mtu by checking if the backend
> > > supports the 'feature-sg'.  If the backend does support it, keep the
> > > current mtu. Otherwise set it to the default value, which is 1500.
> > > 
> > > This helps the vif-bridge hotplug script to set the mtu size to 9000
> > > while bringing up the guest.
> > 
> > Isn't this functionally the same as
> > http://xenbits.xen.org/XCP/linux-2.6.32.pq.hg?file/043b76e4943c/xen-netback-Allow-setting-of-large-MTU-before-rings.patch
> > or its pv-ops parent https://git.kernel.org/?p=linux/kernel/git/jeremy/xen.git;a=commitdiff;h=bee2eec2355c4bf4e149a426d5e30527162de566
> > (in which case it would seem preferable to pull in that change)?
> 
> Either approach is ok.
> 
> Could one of the changes be backported and applied to the 2.6.18.hg tree?

I think for the sake of not fragmenting any more than necessary a
backport would be preferable to adding new code to the old trees.

Why are you still running 2.6.18 though?

Ian.

Re: [PATCH] set mtu from bridge also on vif interface

[Ian Campbell]

On Mon, 2011-02-07 at 09:25 +0000, Olaf Hering wrote:
> On Mon, Feb 07, Ian Campbell wrote:
> 
> > On Sun, 2011-02-06 at 13:41 +0000, Olaf Hering wrote:
> > > Apply mtu size from bridge interface also in vif interface.
> > > This depends on a kernel change which allows arbitrary mtu sizes until
> > > the frontend driver has connected to the backend driver. Without this
> > > kernel change, the vif mtu size will be limited to 1500 even with this
> > > change to the vif-bridge script.
> > > 
> > > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > 
> > In my experience the bridge will only allow it's MTU to be set to the
> > minimum of all the ports (or 1500ish if there are no ports) and will
> > reset to the new minimum when you add a new port.
> > 
> > So does this patch do anything in the absence of the ability to
> > configure the MTU of a VIF from the toolstack?
> 
> This depends if the bridge interface is part of the Xen toolstack
> configuration or if the bridge is part of the system configuration. In
> my setup the bridge is part of the system configuration and gets its mtu
> from the config of the physical interface, eth0 in my case. So br0 has
> mtu of 9000 already when vif-bridge runs.

When I was playing with this last week I was using an internal bridge
(i.e. no physical interface, because I have no physical jumbo
infrastructure) so the physical interface didn't force the bridge.

> With this change, the vif interfaces follow the system configuration.

ok, that's cool then.

Ian.

Re: [PATCH] netback: allow arbitrary mtu size until frontend connects

[Jan Beulich]

>>> On 07.02.11 at 10:11, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> On Mon, 2011-02-07 at 09:03 +0000, Jan Beulich wrote:
>> >>> On 06.02.11 at 14:42, Olaf Hering <olaf@aepfle.de> wrote:
>> > Allow arbitrary mtu size until the frontend is connected.  Once the
>> > connection is established, adjust mtu by checking if the backend
>> > supports the 'feature-sg'.  If the backend does support it, keep the
>> > current mtu. Otherwise set it to the default value, which is 1500.
>> > 
>> > This helps the vif-bridge hotplug script to set the mtu size to 9000
>> > while bringing up the guest.
>> 
>> Isn't this functionally the same as
>> 
> http://xenbits.xen.org/XCP/linux-2.6.32.pq.hg?file/043b76e4943c/xen-netback-A 
> llow-setting-of-large-MTU-before-rings.patch
>> or its pv-ops parent 
> https://git.kernel.org/?p=linux/kernel/git/jeremy/xen.git;a=commitdiff;h=bee2 
> eec2355c4bf4e149a426d5e30527162de566
>> (in which case it would seem preferable to pull in that change)?
>> 
>> Ian, looking at that patch of yours again, the adjustment to
>> netbk_set_sg() seems a little odd: Why is it necessary to
>> reduce dev->mtu unconditionally (i.e. independent of the
>> value of "data") here?
> 
> Hmm. I don't recall. Seems like a bug looking at it now...
> 
> Luckily for me it appears to have been correct by Paul in
> d532fa93d4eeabbfc0176a6a9a93b0d6ade3f6c4

Indeed, to some part: How would dev->mtu grow again if sg
got re-enabled after having been disabled?

Jan

Re: [PATCH] netback: allow arbitrary mtu size until frontend connects

[Olaf Hering]

On Mon, Feb 07, Ian Campbell wrote:

> Why are you still running 2.6.18 though?

Its the base for any non-pv ops kernel, like SLES etc.

Olaf

Re: [PATCH] netback: allow arbitrary mtu size until frontend connects

[Ian Campbell]

On Mon, 2011-02-07 at 09:49 +0000, Jan Beulich wrote:
> >>> On 07.02.11 at 10:11, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> > On Mon, 2011-02-07 at 09:03 +0000, Jan Beulich wrote:
> >> >>> On 06.02.11 at 14:42, Olaf Hering <olaf@aepfle.de> wrote:
> >> > Allow arbitrary mtu size until the frontend is connected.  Once the
> >> > connection is established, adjust mtu by checking if the backend
> >> > supports the 'feature-sg'.  If the backend does support it, keep the
> >> > current mtu. Otherwise set it to the default value, which is 1500.
> >> > 
> >> > This helps the vif-bridge hotplug script to set the mtu size to 9000
> >> > while bringing up the guest.
> >> 
> >> Isn't this functionally the same as
> >> 
> > http://xenbits.xen.org/XCP/linux-2.6.32.pq.hg?file/043b76e4943c/xen-netback-A 
> > llow-setting-of-large-MTU-before-rings.patch
> >> or its pv-ops parent 
> > https://git.kernel.org/?p=linux/kernel/git/jeremy/xen.git;a=commitdiff;h=bee2 
> > eec2355c4bf4e149a426d5e30527162de566
> >> (in which case it would seem preferable to pull in that change)?
> >> 
> >> Ian, looking at that patch of yours again, the adjustment to
> >> netbk_set_sg() seems a little odd: Why is it necessary to
> >> reduce dev->mtu unconditionally (i.e. independent of the
> >> value of "data") here?
> > 
> > Hmm. I don't recall. Seems like a bug looking at it now...
> > 
> > Luckily for me it appears to have been correct by Paul in
> > d532fa93d4eeabbfc0176a6a9a93b0d6ade3f6c4
> 
> Indeed, to some part: How would dev->mtu grow again if sg
> got re-enabled after having been disabled?

Er, manual intervention?

For other features (e.g. gso, csum) I think we track the requested
configuration separately from the currently in force configuration.
Presumably we could do the same here if that's what we want.

Ian.

Re: [PATCH] netback: allow arbitrary mtu size until frontend connects

[Olaf Hering]

On Mon, Feb 07, Ian Campbell wrote:

> I think for the sake of not fragmenting any more than necessary a
> backport would be preferable to adding new code to the old trees.


Allow arbitrary mtu size until the frontend is connected.  Once the
connection is established, adjust mtu by checking if the backend
supports the 'feature-sg'.  If the backend does support it, keep the
current mtu. Otherwise set it to the default value, which is 1500.

based on two commits from
https://git.kernel.org/?p=linux/kernel/git/jeremy/xen.git

bee2eec2355c4bf4e149a426d5e30527162de566

This allows large MTU to be configured by the VIF hotplug
script. Previously this would fail because at the point the hotplug
script runs the VIF features have most likely not been negotiated with
the frontend and so SG has not yet been enabled. Invert this behaviour
so that SG is assumed present until negotiations prove otherwise and
reduce MTU at that point.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

d532fa93d4eeabbfc0176a6a9a93b0d6ade3f6c4

Make sure that if a feature flag is disabled by ethtool on netback
that we do not gratuitously re-enabled it when we check the frontend
features during ring connection.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 drivers/xen/netback/common.h    |   12 +++++--
 drivers/xen/netback/interface.c |   64 +++++++++++++++++++++++++++++++++-------
 drivers/xen/netback/netback.c   |    2 -
 drivers/xen/netback/xenbus.c    |   29 +++++++-----------
 4 files changed, 76 insertions(+), 31 deletions(-)

Index: linux-2.6.18-xen.hg/drivers/xen/netback/common.h
===================================================================
--- linux-2.6.18-xen.hg.orig/drivers/xen/netback/common.h
+++ linux-2.6.18-xen.hg/drivers/xen/netback/common.h
@@ -75,8 +75,13 @@ typedef struct netif_st {
 	struct vm_struct *tx_comms_area;
 	struct vm_struct *rx_comms_area;
 
-	/* Set of features that can be turned on in dev->features. */
-	int features;
+	/* Flags that must not be set in dev->features */
+	int features_disabled;
+
+	/* Frontend feature information. */
+	u8 can_sg:1;
+	u8 gso:1;
+	u8 csum:1;
 
 	/* Internal feature information. */
 	u8 can_queue:1;	/* can queue packets for receiver? */
@@ -182,6 +187,7 @@ void netif_accel_init(void);
 
 void netif_disconnect(netif_t *netif);
 
+void netif_set_features(netif_t *netif);
 netif_t *netif_alloc(struct device *parent, domid_t domid, unsigned int handle);
 int netif_map(netif_t *netif, unsigned long tx_ring_ref,
 	      unsigned long rx_ring_ref, unsigned int evtchn);
@@ -214,7 +220,7 @@ static inline int netbk_can_queue(struct
 static inline int netbk_can_sg(struct net_device *dev)
 {
 	netif_t *netif = netdev_priv(dev);
-	return netif->features & NETIF_F_SG;
+	return netif->can_sg;
 }
 
 #endif /* __NETIF__BACKEND__COMMON_H__ */
Index: linux-2.6.18-xen.hg/drivers/xen/netback/interface.c
===================================================================
--- linux-2.6.18-xen.hg.orig/drivers/xen/netback/interface.c
+++ linux-2.6.18-xen.hg/drivers/xen/netback/interface.c
@@ -93,28 +93,69 @@ static int netbk_change_mtu(struct net_d
 	return 0;
 }
 
-static int netbk_set_sg(struct net_device *dev, u32 data)
+void netif_set_features(netif_t *netif)
+{
+	struct net_device *dev = netif->dev;
+	int features = dev->features;
+
+	if (netif->can_sg)
+		features |= NETIF_F_SG;
+	if (netif->gso)
+		features |= NETIF_F_TSO;
+	if (netif->csum)
+		features |= NETIF_F_IP_CSUM;
+
+	features &= ~(netif->features_disabled);
+
+	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN)
+		dev->mtu = ETH_DATA_LEN;
+
+	dev->features = features;
+}
+
+static int netbk_set_tx_csum(struct net_device *dev, u32 data)
 {
+	netif_t *netif = netdev_priv(dev);
 	if (data) {
-		netif_t *netif = netdev_priv(dev);
+		if (!netif->csum)
+			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_IP_CSUM;
+	} else {
+		netif->features_disabled |= NETIF_F_IP_CSUM;
+	}
 
-		if (!(netif->features & NETIF_F_SG))
+	netif_set_features(netif);
+	return 0;
+}
+
+static int netbk_set_sg(struct net_device *dev, u32 data)
+{
+	netif_t *netif = netdev_priv(dev);
+	if (data) {
+		if (!netif->can_sg)
 			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_SG;
+	} else {
+		netif->features_disabled |= NETIF_F_SG;
 	}
 
-	return ethtool_op_set_sg(dev, data);
+	netif_set_features(netif);
+	return 0;
 }
 
 static int netbk_set_tso(struct net_device *dev, u32 data)
 {
+	netif_t *netif = netdev_priv(dev);
 	if (data) {
-		netif_t *netif = netdev_priv(dev);
-
-		if (!(netif->features & NETIF_F_TSO))
+		if (!netif->gso)
 			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_TSO;
+	} else {
+		netif->features_disabled |= NETIF_F_TSO;
 	}
 
-	return ethtool_op_set_tso(dev, data);
+	netif_set_features(netif);
+	return 0;
 }
 
 static void netbk_get_drvinfo(struct net_device *dev,
@@ -164,7 +205,7 @@ static struct ethtool_ops network_ethtoo
 	.get_drvinfo = netbk_get_drvinfo,
 
 	.get_tx_csum = ethtool_op_get_tx_csum,
-	.set_tx_csum = ethtool_op_set_tx_csum,
+	.set_tx_csum = netbk_set_tx_csum,
 	.get_sg = ethtool_op_get_sg,
 	.set_sg = netbk_set_sg,
 	.get_tso = ethtool_op_get_tso,
@@ -196,6 +237,8 @@ netif_t *netif_alloc(struct device *pare
 	memset(netif, 0, sizeof(*netif));
 	netif->domid  = domid;
 	netif->handle = handle;
+	netif->can_sg = 1;
+	netif->csum = 1;
 	atomic_set(&netif->refcnt, 1);
 	init_waitqueue_head(&netif->waiting_to_free);
 	netif->dev = dev;
@@ -215,7 +258,8 @@ netif_t *netif_alloc(struct device *pare
 	dev->open            = net_open;
 	dev->stop            = net_close;
 	dev->change_mtu	     = netbk_change_mtu;
-	dev->features        = NETIF_F_IP_CSUM;
+
+	netif_set_features(netif);
 
 	SET_ETHTOOL_OPS(dev, &network_ethtool_ops);
 
Index: linux-2.6.18-xen.hg/drivers/xen/netback/netback.c
===================================================================
--- linux-2.6.18-xen.hg.orig/drivers/xen/netback/netback.c
+++ linux-2.6.18-xen.hg/drivers/xen/netback/netback.c
@@ -269,7 +269,7 @@ static struct sk_buff *netbk_copy_skb(st
 
 static inline int netbk_max_required_rx_slots(netif_t *netif)
 {
-	if (netif->features & (NETIF_F_SG|NETIF_F_TSO))
+	if (netif->can_sg || netif->gso)
 		return MAX_SKB_FRAGS + 2; /* header + extra_info + frags */
 	return 1; /* all in one */
 }
Index: linux-2.6.18-xen.hg/drivers/xen/netback/xenbus.c
===================================================================
--- linux-2.6.18-xen.hg.orig/drivers/xen/netback/xenbus.c
+++ linux-2.6.18-xen.hg/drivers/xen/netback/xenbus.c
@@ -356,6 +356,7 @@ static void connect(struct backend_info
 
 static int connect_rings(struct backend_info *be)
 {
+	netif_t *netif = be->netif;
 	struct xenbus_device *dev = be->dev;
 	unsigned long tx_ring_ref, rx_ring_ref;
 	unsigned int evtchn, rx_copy;
@@ -386,44 +387,38 @@ static int connect_rings(struct backend_
 				 dev->otherend);
 		return err;
 	}
-	be->netif->copying_receiver = !!rx_copy;
+	netif->copying_receiver = !!rx_copy;
 
-	if (be->netif->dev->tx_queue_len != 0) {
+	if (netif->dev->tx_queue_len != 0) {
 		if (xenbus_scanf(XBT_NIL, dev->otherend,
 				 "feature-rx-notify", "%d", &val) < 0)
 			val = 0;
 		if (val)
-			be->netif->can_queue = 1;
+			netif->can_queue = 1;
 		else
 			/* Must be non-zero for pfifo_fast to work. */
-			be->netif->dev->tx_queue_len = 1;
+			netif->dev->tx_queue_len = 1;
 	}
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg", "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features |= NETIF_F_SG;
-		be->netif->dev->features |= NETIF_F_SG;
-	}
+	netif->can_sg = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4", "%d",
 			 &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features |= NETIF_F_TSO;
-		be->netif->dev->features |= NETIF_F_TSO;
-	}
+	netif->gso = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-offload",
 			 "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features &= ~NETIF_F_IP_CSUM;
-		be->netif->dev->features &= ~NETIF_F_IP_CSUM;
-	}
+	netif->csum = !val;
+
+	/* Set dev->features */
+	netif_set_features(netif);
 
 	/* Map the shared frame, irq etc. */
-	err = netif_map(be->netif, tx_ring_ref, rx_ring_ref, evtchn);
+	err = netif_map(netif, tx_ring_ref, rx_ring_ref, evtchn);
 	if (err) {
 		xenbus_dev_fatal(dev, err,
 				 "mapping shared-frames %lu/%lu port %u",